Skip to content
This repository has been archived by the owner on Dec 8, 2024. It is now read-only.

Time Parsing fix for #EXT-X-PROGRAM-DATE-TIME #78

Merged
merged 6 commits into from
Mar 19, 2017
Merged
Show file tree
Hide file tree
Changes from 4 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
34 changes: 33 additions & 1 deletion reader.go
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,12 @@ import (

var reKeyValue = regexp.MustCompile(`([a-zA-Z_-]+)=("[^"]+"|[^",]+)`)

// Allow globally apply and/or override Time Parser function.
// Available variants:
// * FullTimeParse - implements full featured ISO/IEC 8601:2004
// * StrictTimeParse - implements only RFC3339 Nanoseconds format
var TimeParse func(value string) (time.Time, error) = FullTimeParse

// Decode parses a master playlist passed from the buffer. If `strict`
// parameter is true then it returns first syntax error.
func (p *MasterPlaylist) Decode(data bytes.Buffer, strict bool) error {
Expand Down Expand Up @@ -501,9 +507,11 @@ func decodeLineOfMediaPlaylist(p *MediaPlaylist, wv *WV, state *decodingState, l
case !state.tagProgramDateTime && strings.HasPrefix(line, "#EXT-X-PROGRAM-DATE-TIME:"):
state.tagProgramDateTime = true
state.listType = MEDIA
if state.programDateTime, err = time.Parse(DATETIME, line[25:]); strict && err != nil {

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can we remove this newline?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

done

if state.programDateTime, err = TimeParse(line[25:]); strict && err != nil {
return err
}

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm unsure why there's new lines here, can these be removed?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is not me, it is my formatter in Sublime Go :)
but sure i can remove it through console editor

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

done

case !state.tagRange && strings.HasPrefix(line, "#EXT-X-BYTERANGE:"):
state.tagRange = true
state.listType = MEDIA
Expand Down Expand Up @@ -638,3 +646,27 @@ func decodeLineOfMediaPlaylist(p *MediaPlaylist, wv *WV, state *decodingState, l
}
return err
}

// Strict Time Wrapper implements RFC3339 with Nanoseconds accuracy
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should be // StrictTimeParse implements.... with a trailing period.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

done

func StrictTimeParse(value string) (time.Time, error) {
return time.Parse(DATETIME, value)
}

// Custom Time Parser implements ISO/IEC 8601:2004
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should be // FileTimeParse implements... with a trailing period.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

done

func FullTimeParse(value string) (time.Time, error) {
layouts := []string{
"2006-01-02T15:04:05.999999999Z0700",
"2006-01-02T15:04:05.999999999Z07:00",
"2006-01-02T15:04:05.999999999Z07",
}
var (
err error
t time.Time
)
for _, layout := range layouts {
if t, err = time.Parse(layout, value); err == nil {
return t, nil
}
}
return t, err
}
161 changes: 161 additions & 0 deletions reader_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -296,6 +296,167 @@ func TestDecodeMediaPlaylistAutoDetectExtend(t *testing.T) {
}
}

// Tests for Time Parsing of EXT-X-PROGRAM-DATE-TIME
// We testing ISO/IEC 8601:2004 where we can get time in UTC, UTC with Nanoseconds
// timeZone in formats '±00:00', '±0000', '±00'
// m3u8.FullTimeParse()
func TestTimeLayoutsDecode(t *testing.T) {
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This test is really just testing the FullTimeParse function, but it's testing it via decodeLineOfMediaPlaylist, I'd just call this function directly.

Also, there's a lot of repetition in the tests, can you rewrite using https://github.com/golang/go/wiki/TableDrivenTests (or sub tests) similar to

m3u8/writer_test.go

Lines 223 to 253 in f55aa7a

// Create new media playlist
// Add segment to media playlist
// Set encryption key
func TestSetKeyForMediaPlaylist(t *testing.T) {
tests := []struct {
KeyFormat string
KeyFormatVersions string
ExpectVersion uint8
}{
{"", "", 3},
{"Format", "", 5},
{"", "Version", 5},
{"Format", "Version", 5},
}
for _, test := range tests {
p, e := NewMediaPlaylist(3, 5)
if e != nil {
t.Fatalf("Create media playlist failed: %s", e)
}
if e = p.Append("test01.ts", 5.0, ""); e != nil {
t.Errorf("Add 1st segment to a media playlist failed: %s", e)
}
if e := p.SetKey("AES-128", "https://example.com", "iv", test.KeyFormat, test.KeyFormatVersions); e != nil {
t.Errorf("Set key to a media playlist failed: %s", e)
}
if p.ver != test.ExpectVersion {
t.Errorf("Set key playlist version: %v, expected: %v", p.ver, test.ExpectVersion)
}
}
}
?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

done

time_in_utc := "#EXT-X-PROGRAM-DATE-TIME:2006-01-02T15:04:05Z"
time_in_utc_nano := "#EXT-X-PROGRAM-DATE-TIME:2006-01-02T15:04:05.123456789Z"
time_with_positive_zone_and_colon := "#EXT-X-PROGRAM-DATE-TIME:2006-01-02T15:04:05+01:00"
time_with_positive_zone_no_colon := "#EXT-X-PROGRAM-DATE-TIME:2006-01-02T15:04:05+0100"
time_with_positive_zone_2digits := "#EXT-X-PROGRAM-DATE-TIME:2006-01-02T15:04:05+01"
time_with_negative_zone_and_colon := "#EXT-X-PROGRAM-DATE-TIME:2006-01-02T15:04:05-01:00"
time_with_negative_zone_no_colon := "#EXT-X-PROGRAM-DATE-TIME:2006-01-02T15:04:05-0100"
time_with_negative_zone_2digits := "#EXT-X-PROGRAM-DATE-TIME:2006-01-02T15:04:05-01"

p, e := NewMediaPlaylist(1, 2)
if e != nil {
t.Fatalf("Create media playlist failed: %s", e)
}
state := new(decodingState)
wv := new(WV)

var err error
err = decodeLineOfMediaPlaylist(p, wv, state, time_in_utc, true)
if err != nil {
t.Errorf("Time Parser Error for %s: %s", time_in_utc, err)
}

state = new(decodingState)
wv = new(WV)
err = decodeLineOfMediaPlaylist(p, wv, state, time_in_utc_nano, true)
if err != nil {
t.Errorf("Time Parser Error for %s: %s", time_in_utc_nano, err)
}

state = new(decodingState)
wv = new(WV)
err = decodeLineOfMediaPlaylist(p, wv, state, time_with_positive_zone_and_colon, true)
if err != nil {
t.Errorf("Time Parser Error for %s: %s", time_with_positive_zone_and_colon, err)
}

state = new(decodingState)
wv = new(WV)
err = decodeLineOfMediaPlaylist(p, wv, state, time_with_positive_zone_no_colon, true)
if err != nil {
t.Errorf("Time Parser Error for %s: %s", time_with_positive_zone_no_colon, err)
}

state = new(decodingState)
wv = new(WV)
err = decodeLineOfMediaPlaylist(p, wv, state, time_with_positive_zone_2digits, true)
if err != nil {
t.Errorf("Time Parser Error for %s: %s", time_with_positive_zone_2digits, err)
}

state = new(decodingState)
wv = new(WV)
err = decodeLineOfMediaPlaylist(p, wv, state, time_with_negative_zone_and_colon, true)
if err != nil {
t.Errorf("Time Parser Error for %s: %s", time_with_negative_zone_and_colon, err)
}

state = new(decodingState)
wv = new(WV)
err = decodeLineOfMediaPlaylist(p, wv, state, time_with_negative_zone_no_colon, true)
if err != nil {
t.Errorf("Time Parser Error for %s: %s", time_with_negative_zone_no_colon, err)
}

state = new(decodingState)
wv = new(WV)
err = decodeLineOfMediaPlaylist(p, wv, state, time_with_negative_zone_2digits, true)
if err != nil {
t.Errorf("Time Parser Error for %s: %s", time_with_negative_zone_2digits, err)
}
}

// Tests for Time Parsing of EXT-X-PROGRAM-DATE-TIME
// We testing Strict format of RFC3339 where we can get time in UTC, UTC with Nanoseconds
// timeZone in formats '±00:00', '±0000', '±00'
// m3u8.StrictTimeParse()
func TestStrictTimeLayoutsDecode(t *testing.T) {
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This test is really just testing the StrictTimeParse function, but it's testing it via decodeLineOfMediaPlaylist, I'd just call this function directly.

Also, there's a lot of repetition in the tests, can you rewrite using https://github.com/golang/go/wiki/TableDrivenTests (or sub tests) similar to

m3u8/writer_test.go

Lines 223 to 253 in f55aa7a

// Create new media playlist
// Add segment to media playlist
// Set encryption key
func TestSetKeyForMediaPlaylist(t *testing.T) {
tests := []struct {
KeyFormat string
KeyFormatVersions string
ExpectVersion uint8
}{
{"", "", 3},
{"Format", "", 5},
{"", "Version", 5},
{"Format", "Version", 5},
}
for _, test := range tests {
p, e := NewMediaPlaylist(3, 5)
if e != nil {
t.Fatalf("Create media playlist failed: %s", e)
}
if e = p.Append("test01.ts", 5.0, ""); e != nil {
t.Errorf("Add 1st segment to a media playlist failed: %s", e)
}
if e := p.SetKey("AES-128", "https://example.com", "iv", test.KeyFormat, test.KeyFormatVersions); e != nil {
t.Errorf("Set key to a media playlist failed: %s", e)
}
if p.ver != test.ExpectVersion {
t.Errorf("Set key playlist version: %v, expected: %v", p.ver, test.ExpectVersion)
}
}
}
?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ok will check links and re-do tests.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

done


// Should Pass
time_in_utc := "#EXT-X-PROGRAM-DATE-TIME:2006-01-02T15:04:05Z"
time_in_utc_nano := "#EXT-X-PROGRAM-DATE-TIME:2006-01-02T15:04:05.123456789Z"
time_with_positive_zone_and_colon := "#EXT-X-PROGRAM-DATE-TIME:2006-01-02T15:04:05+01:00"
time_with_negative_zone_and_colon := "#EXT-X-PROGRAM-DATE-TIME:2006-01-02T15:04:05-01:00"

// Should Fail
time_with_positive_zone_no_colon := "#EXT-X-PROGRAM-DATE-TIME:2006-01-02T15:04:05+0100"
time_with_positive_zone_2digits := "#EXT-X-PROGRAM-DATE-TIME:2006-01-02T15:04:05+01"
time_with_negative_zone_no_colon := "#EXT-X-PROGRAM-DATE-TIME:2006-01-02T15:04:05-0100"
time_with_negative_zone_2digits := "#EXT-X-PROGRAM-DATE-TIME:2006-01-02T15:04:05-01"

// Set TimeParse function to StrictTimeParse (RFC3339 Only)
TimeParse = StrictTimeParse

p, e := NewMediaPlaylist(1, 2)
if e != nil {
t.Fatalf("Create media playlist failed: %s", e)
}
state := new(decodingState)
wv := new(WV)

var err error
err = decodeLineOfMediaPlaylist(p, wv, state, time_in_utc, true)
if err != nil {
t.Errorf("Time Parser Error for %s: %s", time_in_utc, err)
}

state = new(decodingState)
wv = new(WV)
err = decodeLineOfMediaPlaylist(p, wv, state, time_in_utc_nano, true)
if err != nil {
t.Errorf("Time Parser Error for %s: %s", time_in_utc_nano, err)
}

state = new(decodingState)
wv = new(WV)
err = decodeLineOfMediaPlaylist(p, wv, state, time_with_positive_zone_and_colon, true)
if err != nil {
t.Errorf("Time Parser Error for %s: %s", time_with_positive_zone_and_colon, err)
}

state = new(decodingState)
wv = new(WV)
err = decodeLineOfMediaPlaylist(p, wv, state, time_with_positive_zone_no_colon, true)
if err != nil {
t.Logf("Time Parser Error for %s: %s", time_with_positive_zone_no_colon, err)
}

state = new(decodingState)
wv = new(WV)
err = decodeLineOfMediaPlaylist(p, wv, state, time_with_positive_zone_2digits, true)
if err != nil {
t.Logf("Time Parser Error for %s: %s", time_with_positive_zone_2digits, err)
}

state = new(decodingState)
wv = new(WV)
err = decodeLineOfMediaPlaylist(p, wv, state, time_with_negative_zone_and_colon, true)
if err != nil {
t.Errorf("Time Parser Error for %s: %s", time_with_negative_zone_and_colon, err)
}

state = new(decodingState)
wv = new(WV)
err = decodeLineOfMediaPlaylist(p, wv, state, time_with_negative_zone_no_colon, true)
if err != nil {
t.Logf("Time Parser Error for %s: %s", time_with_negative_zone_no_colon, err)
}

state = new(decodingState)
wv = new(WV)
err = decodeLineOfMediaPlaylist(p, wv, state, time_with_negative_zone_2digits, true)
if err != nil {
t.Logf("Time Parser Error for %s: %s", time_with_negative_zone_2digits, err)
}
}

/***************************
* Code parsing examples *
***************************/
Expand Down
4 changes: 3 additions & 1 deletion structure.go
Original file line number Diff line number Diff line change
Expand Up @@ -31,8 +31,10 @@ const (
o The EXT-X-MEDIA tag.
o The AUDIO and VIDEO attributes of the EXT-X-STREAM-INF tag.
*/
minver = uint8(3)
minver = uint8(3)

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can we remove this newline?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

done

DATETIME = time.RFC3339Nano // Format for EXT-X-PROGRAM-DATE-TIME defined in section 3.4.5

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm unsure why there's new lines here, can these be removed?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

same as first time, SublimeGo formatter

)

type ListType uint
Expand Down