From 6d9953141be04bca61b689282e03d08b76c7e8c7 Mon Sep 17 00:00:00 2001 From: sam-eng Date: Wed, 5 Feb 2020 16:55:56 -0500 Subject: [PATCH 1/6] add new test for master playlists with alternatives --- reader_test.go | 30 ++++++++++++++++++- .../master-with-alternatives-diff-order.m3u8 | 18 +++++++++++ 2 files changed, 47 insertions(+), 1 deletion(-) create mode 100644 sample-playlists/master-with-alternatives-diff-order.m3u8 diff --git a/reader_test.go b/reader_test.go index 8d60b16c..6d567b80 100644 --- a/reader_test.go +++ b/reader_test.go @@ -86,7 +86,7 @@ func TestDecodeMasterPlaylistWithAlternatives(t *testing.T) { } // TODO check other values for i, v := range p.Variants { - if i == 0 && len(v.Alternatives) != 3 { + if i == 0 && len(v.Alternatives) != 9 { t.Fatalf("not all alternatives from #EXT-X-MEDIA parsed (has %d but should be 3", len(v.Alternatives)) } if i == 1 && len(v.Alternatives) != 3 { @@ -102,6 +102,34 @@ func TestDecodeMasterPlaylistWithAlternatives(t *testing.T) { // fmt.Println(p.Encode().String()) } +func TestDecodeMasterPlaylistWithAlternativesDifferentOrder(t *testing.T) { + f, err := os.Open("sample-playlists/master-with-alternatives-diff-order.m3u8") + if err != nil { + t.Fatal(err) + } + p := NewMasterPlaylist() + err = p.DecodeFrom(bufio.NewReader(f), false) + if err != nil { + t.Fatal(err) + } + // check parsed values + if p.ver != 3 { + t.Errorf("Version of parsed playlist = %d (must = 3)", p.ver) + } + if len(p.Variants) != 4 { + t.Fatal("not all variants in master playlist parsed") + } + // TODO check other values + for _, v := range p.Variants { + if len(v.Alternatives) != 9 { + t.Errorf("not all alternatives from #EXT-X-MEDIA parsed (has %d but should be 9).\nThis variant is the problem: %v", len(v.Alternatives), v) + } else { + fmt.Printf("SUCCESS! This variant passed: %v", v) + } + } + // fmt.Println(p.Encode().String()) +} + func TestDecodeMasterPlaylistWithClosedCaptionEqNone(t *testing.T) { f, err := os.Open("sample-playlists/master-with-closed-captions-eq-none.m3u8") if err != nil { diff --git a/sample-playlists/master-with-alternatives-diff-order.m3u8 b/sample-playlists/master-with-alternatives-diff-order.m3u8 new file mode 100644 index 00000000..cd483bf8 --- /dev/null +++ b/sample-playlists/master-with-alternatives-diff-order.m3u8 @@ -0,0 +1,18 @@ +#EXTM3U +#EXT-X-MEDIA:TYPE=VIDEO,GROUP-ID="low",NAME="Main",DEFAULT=YES,URI="low/main/audio-video.m3u8" +#EXT-X-MEDIA:TYPE=VIDEO,GROUP-ID="low",NAME="Centerfield",DEFAULT=NO,URI="low/centerfield/audio-video.m3u8" +#EXT-X-MEDIA:TYPE=VIDEO,GROUP-ID="low",NAME="Dugout",DEFAULT=NO,URI="low/dugout/audio-video.m3u8" +#EXT-X-MEDIA:TYPE=VIDEO,GROUP-ID="mid",NAME="Main",DEFAULT=YES,URI="mid/main/audio-video.m3u8" +#EXT-X-MEDIA:TYPE=VIDEO,GROUP-ID="mid",NAME="Centerfield",DEFAULT=NO,URI="mid/centerfield/audio-video.m3u8" +#EXT-X-MEDIA:TYPE=VIDEO,GROUP-ID="mid",NAME="Dugout",DEFAULT=NO,URI="mid/dugout/audio-video.m3u8" +#EXT-X-MEDIA:TYPE=VIDEO,GROUP-ID="hi",NAME="Main",DEFAULT=YES,URI="hi/main/audio-video.m3u8" +#EXT-X-MEDIA:TYPE=VIDEO,GROUP-ID="hi",NAME="Centerfield",DEFAULT=NO,URI="hi/centerfield/audio-video.m3u8" +#EXT-X-MEDIA:TYPE=VIDEO,GROUP-ID="hi",NAME="Dugout",DEFAULT=NO,URI="hi/dugout/audio-video.m3u8" +#EXT-X-STREAM-INF:BANDWIDTH=1280000,VIDEO="low" +low/main/audio-video.m3u8 +#EXT-X-STREAM-INF:BANDWIDTH=2560000,VIDEO="mid" +mid/main/audio-video.m3u8 +#EXT-X-STREAM-INF:BANDWIDTH=7680000,VIDEO="hi" +hi/main/audio-video.m3u8 +#EXT-X-STREAM-INF:BANDWIDTH=65000,CODECS="mp4a.40.5" +main/audio-only.m3u8 From 211387e85b614976876927518196a3dde0ce9986 Mon Sep 17 00:00:00 2001 From: sam-eng Date: Wed, 5 Feb 2020 18:29:30 -0500 Subject: [PATCH 2/6] implement basic fix to the issue --- reader.go | 52 +++++++++++++++++++++++++++++++++++++++++--------- reader_test.go | 22 +++++++++++++-------- structure.go | 2 +- 3 files changed, 58 insertions(+), 18 deletions(-) diff --git a/reader.go b/reader.go index b19324eb..ddb16d90 100644 --- a/reader.go +++ b/reader.go @@ -65,6 +65,7 @@ func (p *MasterPlaylist) decode(buf *bytes.Buffer, strict bool) error { var eof bool state := new(decodingState) + state.alternatives = make(map[string][]*Alternative) for !eof { line, err := buf.ReadString('\n') @@ -81,6 +82,46 @@ func (p *MasterPlaylist) decode(buf *bytes.Buffer, strict bool) error { if strict && !state.m3u { return errors.New("#EXTM3U absent") } + // Do something to handle variants and their alternatives + for _, variant := range p.Variants { + alts := []*Alternative{} + + if variant.Audio != "" { + toSearch := state.alternatives["AUDIO"] + for _, alt := range toSearch { + if variant.Audio == alt.GroupId { + alts = append(alts, alt) + } + } + } + if variant.Video != "" { + toSearch := state.alternatives["VIDEO"] + for _, alt := range toSearch { + if variant.Video == alt.GroupId { + alts = append(alts, alt) + } + } + } + if variant.Subtitles != "" { + toSearch := state.alternatives["SUBTITLES"] + for _, alt := range toSearch { + if variant.Subtitles == alt.GroupId { + alts = append(alts, alt) + } + } + } + if variant.Captions != "" { + toSearch := state.alternatives["CLOSED-CAPTIONS"] + for _, alt := range toSearch { + if variant.Captions == alt.GroupId { + alts = append(alts, alt) + } + } + } + + variant.Alternatives = alts + } + return nil } @@ -240,6 +281,7 @@ func decode(buf *bytes.Buffer, strict bool, customDecoders []CustomDecoder) (Pla switch state.listType { case MASTER: + // Sam: Do something to handle variants and their alternatives return master, MASTER, nil case MEDIA: if media.Closed || media.MediaType == EVENT { @@ -331,15 +373,11 @@ func decodeLineOfMasterPlaylist(p *MasterPlaylist, state *decodingState, line st alt.URI = v } } - state.alternatives = append(state.alternatives, &alt) + state.alternatives[alt.Type] = append(state.alternatives[alt.Type], &alt) case !state.tagStreamInf && strings.HasPrefix(line, "#EXT-X-STREAM-INF:"): state.tagStreamInf = true state.listType = MASTER state.variant = new(Variant) - if len(state.alternatives) > 0 { - state.variant.Alternatives = state.alternatives - state.alternatives = nil - } p.Variants = append(p.Variants, state.variant) for k, v := range decodeParamsLine(line[18:]) { switch k { @@ -395,10 +433,6 @@ func decodeLineOfMasterPlaylist(p *MasterPlaylist, state *decodingState, line st state.listType = MASTER state.variant = new(Variant) state.variant.Iframe = true - if len(state.alternatives) > 0 { - state.variant.Alternatives = state.alternatives - state.alternatives = nil - } p.Variants = append(p.Variants, state.variant) for k, v := range decodeParamsLine(line[26:]) { switch k { diff --git a/reader_test.go b/reader_test.go index 6d567b80..74f45301 100644 --- a/reader_test.go +++ b/reader_test.go @@ -86,7 +86,7 @@ func TestDecodeMasterPlaylistWithAlternatives(t *testing.T) { } // TODO check other values for i, v := range p.Variants { - if i == 0 && len(v.Alternatives) != 9 { + if i == 0 && len(v.Alternatives) != 3 { t.Fatalf("not all alternatives from #EXT-X-MEDIA parsed (has %d but should be 3", len(v.Alternatives)) } if i == 1 && len(v.Alternatives) != 3 { @@ -119,15 +119,21 @@ func TestDecodeMasterPlaylistWithAlternativesDifferentOrder(t *testing.T) { if len(p.Variants) != 4 { t.Fatal("not all variants in master playlist parsed") } - // TODO check other values - for _, v := range p.Variants { - if len(v.Alternatives) != 9 { - t.Errorf("not all alternatives from #EXT-X-MEDIA parsed (has %d but should be 9).\nThis variant is the problem: %v", len(v.Alternatives), v) - } else { - fmt.Printf("SUCCESS! This variant passed: %v", v) + // TODO add check ensuring the right alternatives are in the variant (switch from len base to all match the group-id) + for i, v := range p.Variants { + if i == 0 && len(v.Alternatives) != 3 { + t.Fatalf("not all alternatives from #EXT-X-MEDIA parsed (has %d but should be 3)", len(v.Alternatives)) + } + if i == 1 && len(v.Alternatives) != 3 { + t.Fatalf("not all alternatives from #EXT-X-MEDIA parsed (has %d but should be 3)", len(v.Alternatives)) + } + if i == 2 && len(v.Alternatives) != 3 { + t.Fatalf("not all alternatives from #EXT-X-MEDIA parsed (has %d but should be 3)", len(v.Alternatives)) + } + if i == 3 && len(v.Alternatives) > 0 { + t.Fatal("should not be alternatives for this variant") } } - // fmt.Println(p.Encode().String()) } func TestDecodeMasterPlaylistWithClosedCaptionEqNone(t *testing.T) { diff --git a/structure.go b/structure.go index eb5d01a2..b05ef2e4 100644 --- a/structure.go +++ b/structure.go @@ -327,7 +327,7 @@ type decodingState struct { duration float64 title string variant *Variant - alternatives []*Alternative + alternatives map[string][]*Alternative xkey *Key xmap *Map scte *SCTE From 4ed0b1c69fed968369582f941966089b472055df Mon Sep 17 00:00:00 2001 From: sam-eng Date: Fri, 7 Feb 2020 11:38:14 -0500 Subject: [PATCH 3/6] add more tests --- reader_test.go | 25 +++++++---- .../master-with-alternatives-diff-order.m3u8 | 42 +++++++++++++++++-- 2 files changed, 56 insertions(+), 11 deletions(-) diff --git a/reader_test.go b/reader_test.go index 74f45301..0a494381 100644 --- a/reader_test.go +++ b/reader_test.go @@ -116,22 +116,31 @@ func TestDecodeMasterPlaylistWithAlternativesDifferentOrder(t *testing.T) { if p.ver != 3 { t.Errorf("Version of parsed playlist = %d (must = 3)", p.ver) } - if len(p.Variants) != 4 { + if len(p.Variants) != 7 { t.Fatal("not all variants in master playlist parsed") } // TODO add check ensuring the right alternatives are in the variant (switch from len base to all match the group-id) for i, v := range p.Variants { if i == 0 && len(v.Alternatives) != 3 { - t.Fatalf("not all alternatives from #EXT-X-MEDIA parsed (has %d but should be 3)", len(v.Alternatives)) + t.Errorf("Expect 3 alternatives at %d but got %d\n", i, len(v.Alternatives)) } - if i == 1 && len(v.Alternatives) != 3 { - t.Fatalf("not all alternatives from #EXT-X-MEDIA parsed (has %d but should be 3)", len(v.Alternatives)) + if i == 1 && len(v.Alternatives) != 4 { + t.Errorf("Expect 4 alternatives at %d but got %d\n", i, len(v.Alternatives)) } - if i == 2 && len(v.Alternatives) != 3 { - t.Fatalf("not all alternatives from #EXT-X-MEDIA parsed (has %d but should be 3)", len(v.Alternatives)) + if i == 2 && len(v.Alternatives) != 4 { + t.Errorf("Expect 4 alternatives at %d but got %d\n", i, len(v.Alternatives)) } - if i == 3 && len(v.Alternatives) > 0 { - t.Fatal("should not be alternatives for this variant") + if i == 3 && len(v.Alternatives) != 1 { + t.Errorf("Expect 1 alternative at %d but got %d\n", i, len(v.Alternatives)) + } + if i == 4 && len(v.Alternatives) != 0 { + t.Errorf("Expect 0 alternatives at %d but got %d\n", i, len(v.Alternatives)) + } + if i == 5 && len(v.Alternatives) != 1 { + t.Errorf("Expect 1 alternative at %d but got %d\n", i, len(v.Alternatives)) + } + if i == 6 && len(v.Alternatives) != 1 { + t.Errorf("Expect 1 alternative at %d but got %d\n", i, len(v.Alternatives)) } } } diff --git a/sample-playlists/master-with-alternatives-diff-order.m3u8 b/sample-playlists/master-with-alternatives-diff-order.m3u8 index cd483bf8..6b9341db 100644 --- a/sample-playlists/master-with-alternatives-diff-order.m3u8 +++ b/sample-playlists/master-with-alternatives-diff-order.m3u8 @@ -1,4 +1,6 @@ #EXTM3U + +# Videos #EXT-X-MEDIA:TYPE=VIDEO,GROUP-ID="low",NAME="Main",DEFAULT=YES,URI="low/main/audio-video.m3u8" #EXT-X-MEDIA:TYPE=VIDEO,GROUP-ID="low",NAME="Centerfield",DEFAULT=NO,URI="low/centerfield/audio-video.m3u8" #EXT-X-MEDIA:TYPE=VIDEO,GROUP-ID="low",NAME="Dugout",DEFAULT=NO,URI="low/dugout/audio-video.m3u8" @@ -8,11 +10,45 @@ #EXT-X-MEDIA:TYPE=VIDEO,GROUP-ID="hi",NAME="Main",DEFAULT=YES,URI="hi/main/audio-video.m3u8" #EXT-X-MEDIA:TYPE=VIDEO,GROUP-ID="hi",NAME="Centerfield",DEFAULT=NO,URI="hi/centerfield/audio-video.m3u8" #EXT-X-MEDIA:TYPE=VIDEO,GROUP-ID="hi",NAME="Dugout",DEFAULT=NO,URI="hi/dugout/audio-video.m3u8" + +# Audios +#EXT-X-MEDIA:TYPE=AUDIO,GROUP-ID="a1",NAME="Audio1",DEFAULT=YES,URI="audio/a1.m3u8" + +# Captions +#EXT-X-MEDIA:TYPE=CLOSED-CAPTIONS,GROUP-ID="cc",LANGUAGE="en",NAME="English",DEFAULT=YES,AUTOSELECT=YES + +# Subtitles +#EXT-X-MEDIA:TYPE=SUBTITLES,GROUP-ID="sub1",LANGUAGE="en",NAME="English",AUTOSELECT=YES,DEFAULT=YES,FORCED=NO,URI="s1/en/prog_index.m3u8" + +# External Media with no usage +#EXT-X-MEDIA:TYPE=SUBTITLES,GROUP-ID="sub2",LANGUAGE="en",NAME="English",AUTOSELECT=YES,DEFAULT=YES,FORCED=NO,URI="s1/en/prog_index.m3u8" + +# Video #EXT-X-STREAM-INF:BANDWIDTH=1280000,VIDEO="low" low/main/audio-video.m3u8 -#EXT-X-STREAM-INF:BANDWIDTH=2560000,VIDEO="mid" + +# Video and CC +#EXT-X-STREAM-INF:BANDWIDTH=2560000,VIDEO="mid",CLOSED-CAPTIONS="cc" mid/main/audio-video.m3u8 -#EXT-X-STREAM-INF:BANDWIDTH=7680000,VIDEO="hi" + +# Video and Subtitles +#EXT-X-STREAM-INF:BANDWIDTH=7680000,VIDEO="hi",SUBTITLES="sub1" hi/main/audio-video.m3u8 -#EXT-X-STREAM-INF:BANDWIDTH=65000,CODECS="mp4a.40.5" + +# Audio +#EXT-X-STREAM-INF:BANDWIDTH=65000,CODECS="mp4a.40.5",AUDIO="a1" main/audio-only.m3u8 + +# Non-existent group-id +#EXT-X-STREAM-INF:BANDWIDTH=2560000,VIDEO="missing" +missing-id/main/audio-video.m3u8 + +# One group-id present and one non-existent group id +#EXT-X-STREAM-INF:BANDWIDTH=2560000,VIDEO="missing",AUDIO="a1" +missing-and-valid-id/main/audio-video.m3u8 + +# Stream coming before external media +#EXT-X-STREAM-INF:BANDWIDTH=2560000,VIDEO="v1" +pre-media/main/audio-video.m3u8 + +#EXT-X-MEDIA:TYPE=VIDEO,GROUP-ID="v1",NAME="AfterStream",DEFAULT=NO,URI="after-stream/audio-video.m3u8" \ No newline at end of file From 84d795e676f396d18072c27e0b473b6dd048ef90 Mon Sep 17 00:00:00 2001 From: sam-eng Date: Mon, 10 Feb 2020 12:30:03 -0500 Subject: [PATCH 4/6] turn fix into its own function --- reader.go | 13 ++++++++++--- reader_test.go | 3 ++- 2 files changed, 12 insertions(+), 4 deletions(-) diff --git a/reader.go b/reader.go index ddb16d90..912f5320 100644 --- a/reader.go +++ b/reader.go @@ -82,7 +82,14 @@ func (p *MasterPlaylist) decode(buf *bytes.Buffer, strict bool) error { if strict && !state.m3u { return errors.New("#EXTM3U absent") } - // Do something to handle variants and their alternatives + + p.setAlternatives(state) + + return nil +} + +// Set alternatives for variants in master playlist. Internal function. +func (p *MasterPlaylist) setAlternatives(state *decodingState) { for _, variant := range p.Variants { alts := []*Alternative{} @@ -121,8 +128,6 @@ func (p *MasterPlaylist) decode(buf *bytes.Buffer, strict bool) error { variant.Alternatives = alts } - - return nil } // Decode parses a media playlist passed from the buffer. If `strict` @@ -231,6 +236,7 @@ func decode(buf *bytes.Buffer, strict bool, customDecoders []CustomDecoder) (Pla var err error state := new(decodingState) + state.alternatives = make(map[string][]*Alternative) // create the map for alternatives wv := new(WV) master = NewMasterPlaylist() @@ -282,6 +288,7 @@ func decode(buf *bytes.Buffer, strict bool, customDecoders []CustomDecoder) (Pla switch state.listType { case MASTER: // Sam: Do something to handle variants and their alternatives + master.setAlternatives(state) return master, MASTER, nil case MEDIA: if media.Closed || media.MediaType == EVENT { diff --git a/reader_test.go b/reader_test.go index 0a494381..15094320 100644 --- a/reader_test.go +++ b/reader_test.go @@ -119,7 +119,7 @@ func TestDecodeMasterPlaylistWithAlternativesDifferentOrder(t *testing.T) { if len(p.Variants) != 7 { t.Fatal("not all variants in master playlist parsed") } - // TODO add check ensuring the right alternatives are in the variant (switch from len base to all match the group-id) + for i, v := range p.Variants { if i == 0 && len(v.Alternatives) != 3 { t.Errorf("Expect 3 alternatives at %d but got %d\n", i, len(v.Alternatives)) @@ -143,6 +143,7 @@ func TestDecodeMasterPlaylistWithAlternativesDifferentOrder(t *testing.T) { t.Errorf("Expect 1 alternative at %d but got %d\n", i, len(v.Alternatives)) } } + } func TestDecodeMasterPlaylistWithClosedCaptionEqNone(t *testing.T) { From 08dfd5cb4e3469a6ed8641b7dd8b365b2c561482 Mon Sep 17 00:00:00 2001 From: sam-eng Date: Mon, 10 Feb 2020 12:32:26 -0500 Subject: [PATCH 5/6] remove extraneous comment --- reader.go | 1 - 1 file changed, 1 deletion(-) diff --git a/reader.go b/reader.go index 912f5320..0d1721f8 100644 --- a/reader.go +++ b/reader.go @@ -287,7 +287,6 @@ func decode(buf *bytes.Buffer, strict bool, customDecoders []CustomDecoder) (Pla switch state.listType { case MASTER: - // Sam: Do something to handle variants and their alternatives master.setAlternatives(state) return master, MASTER, nil case MEDIA: From 2745291629990688fdef69fc95db811b02f2d37a Mon Sep 17 00:00:00 2001 From: sam-eng Date: Mon, 10 Feb 2020 14:31:05 -0500 Subject: [PATCH 6/6] small fix to pass tests --- reader.go | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/reader.go b/reader.go index 0d1721f8..0fcf2bdf 100644 --- a/reader.go +++ b/reader.go @@ -126,7 +126,9 @@ func (p *MasterPlaylist) setAlternatives(state *decodingState) { } } - variant.Alternatives = alts + if len(alts) != 0 { + variant.Alternatives = alts + } } }