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

CLOSED-CAPTIONS quoted-string string fixup #80

Merged
merged 2 commits into from
Mar 8, 2017

Conversation

mbowBC
Copy link
Contributor

@mbowBC mbowBC commented Mar 8, 2017

The value can be either a quoted-string or an enumerated-string

with the value NONE. If the value is a quoted-string, it MUST
match the value of the GROUP-ID attribute of an EXT-X-MEDIA tag
elsewhere in the Playlist whose TYPE attribute is CLOSED-CAPTIONS,
and indicates the set of closed-caption Renditions that can be
used when playing the presentation.

Hi I'm very sorry I missed this in spec that when the value is none it should not use quoted strings.

	The value can be either a quoted-string or an enumerated-string
with the value NONE.  If the value is a quoted-string, it MUST
match the value of the GROUP-ID attribute of an EXT-X-MEDIA tag
elsewhere in the Playlist whose TYPE attribute is CLOSED-CAPTIONS,
and indicates the set of closed-caption Renditions that can be
used when playing the presentation.
@coveralls
Copy link

coveralls commented Mar 8, 2017

Coverage Status

Coverage increased (+0.1%) to 65.149% when pulling 251ea54 on mbowBC:CC=NONE_single_quote_string_fix into e521730 on grafov:master.

Copy link
Collaborator

@bradleyfalzon bradleyfalzon left a comment

Choose a reason for hiding this comment

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

Ah, I misread that also, cheers for the follow up. Two tiny nits if you're able to fix those and I'll merge.

writer_test.go Outdated
@@ -572,10 +572,18 @@ func TestNewMasterPlaylistWithClosedCaptionEqNone(t *testing.T) {
}
m.Append(fmt.Sprintf("eng_rendition_rendition.m3u8"), p, *vp)

expected := `CLOSED-CAPTIONS="NONE"`
expected := `CLOSED-CAPTIONS=NONE`
Copy link
Collaborator

Choose a reason for hiding this comment

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

nit: could just be double quotes now.

writer_test.go Outdated
m2.Append(fmt.Sprintf("eng_rendition_rendition.m3u8"), p, *vp)
expected = `CLOSED-CAPTIONS="CC1"`
if !strings.Contains(m2.String(), expected) {
t.Fatalf("Master playlist did not contain: %s, \nMaster Playlist: \n%v", expected, m2.String())
Copy link
Collaborator

Choose a reason for hiding this comment

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

I'd reformat this to remove the trailing spaces Master playlist did not contain: %s\nMaster Playlist:\n%v

@coveralls
Copy link

coveralls commented Mar 8, 2017

Coverage Status

Coverage increased (+0.1%) to 65.149% when pulling f3ca8ad on mbowBC:CC=NONE_single_quote_string_fix into e521730 on grafov:master.

@bradleyfalzon bradleyfalzon merged commit acec4ae into grafov:master Mar 8, 2017
@mbowBC
Copy link
Contributor Author

mbowBC commented Mar 8, 2017

thanks

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.

None yet

3 participants