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

Segments metadata track is shown when changing source #5553

Closed
johnbyers opened this issue Nov 5, 2018 · 10 comments
Closed

Segments metadata track is shown when changing source #5553

johnbyers opened this issue Nov 5, 2018 · 10 comments

Comments

@johnbyers
Copy link

@johnbyers johnbyers commented Nov 5, 2018

Description

If you had captions on, and then change the video source, you get some weird text on the screen. The guys on videojs slack thought it was the segments metadata track.

Steps to reproduce

  1. Visit the VHS test page at https://videojs.github.io/http-streaming/
  2. Play the video, choosing the very bottom closed caption track, "cc1". (Not sure what language it is for, but the captions show English)
  3. Reset the player's source by clicking the "Load" button again.
  4. Play the video, and see the weird text/json

Results

Expected

No captions after changing source.

Actual

Even though closed captions are set to "off" after switching the source, this appears as a caption over the video:
{"bandwidth":915905,"resolution":{"width":960,"height":540},"codecs":"mpra.40.2,avc1.4d401f","byteLength":1046972,"uri":"https://d2zihajmogu5jn.cloudfront.net/bipbop-advanced/gear3/main.ts","timeline":0,"playlist":"gear3/prog_index.m3u8","start":9.976644444444446,"end":19.91992222222223}

Error output

Additional Information

versions

videojs

I've seen it in 7.2.2 and 7.2.4

browsers

Chrome, Firefox, and IE11.
Edge doesn't even show captions.
Mobile Safari doesn't show that bottom "CC" caption option, so it doesn't seem to have the problem.

OSes

Microsoft Windows 10

plugins

None

@welcome
Copy link

@welcome welcome bot commented Nov 5, 2018

👋 Thanks for opening your first issue here! 👋

If you're reporting a 🐞 bug, please make sure you include steps to reproduce it. We get a lot of issues on this repo, so please be patient and we will get back to you as soon as we can.
To help make it easier for us to investigate your issue, please follow the contributing guidelines.

@gkatsev
Copy link
Member

@gkatsev gkatsev commented Nov 5, 2018

Ooh, thanks for the reproducible test case!

@ldayananda
Copy link
Contributor

@ldayananda ldayananda commented Nov 5, 2018

From taking the quickest of looks at this, I think it's because userPref.language and track.language(for the metadata-track) are both '' here:

userPref.language === track.language) {

We should probably ensure that text-track-display completely ignores non-visible track types

@ldayananda
Copy link
Contributor

@ldayananda ldayananda commented Nov 5, 2018

It might also make sense to clear the TextTrackList queue on source changes too?

https://github.com/videojs/video.js/blame/8c92cbfb3e98259999224d5dd7778f35dfe1cab7/src/js/tracks/text-track-list.js#L31

@gkatsev
Copy link
Member

@gkatsev gkatsev commented Nov 5, 2018

clearing the event queue makes sense.
We should update the preselectTrack method to ignore when language is the empty string and also ignore tracks that are chapters or metadata

@johnbyers
Copy link
Author

@johnbyers johnbyers commented Nov 5, 2018

What is special about that "cc1" caption? All the others have a language, but that one just says "cc1". Is there something different about it?

@gkatsev
Copy link
Member

@gkatsev gkatsev commented Nov 5, 2018

cc1 is the first caption track for 608 captions which live inside the video data. Not sure if that has language data but we should consider setting one for it as well.

@johnbyers
Copy link
Author

@johnbyers johnbyers commented Nov 5, 2018

That is interesting, I thought we were using WebVTT captions in our project, I guess they have turned into 608 captions

@gkatsev
Copy link
Member

@gkatsev gkatsev commented Nov 6, 2018

It looks like any caption without a language would trigger this issue.

@gkatsev
Copy link
Member

@gkatsev gkatsev commented Nov 6, 2018

I have a fix for this in #5556

@gkatsev gkatsev closed this in #5556 Nov 6, 2018
gkatsev added a commit that referenced this issue Nov 6, 2018
… set (#5556)

When preselecting a new track based on user preference, make sure that the language is actually set and that the track we're testing is either a captions or a subtitles track.

Fixes #5553
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Linked pull requests

Successfully merging a pull request may close this issue.

None yet
3 participants