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

Feature webvtt #700

Closed
wants to merge 15 commits into from
Closed

Conversation

coolheinze
Copy link
Contributor

Added WebVTT subtitles feature.

I use vtt.js by Mozilla for the actual parsing of the vtt files, although I had to add code to handle the x-timestamp-map header. This does, however, require having an updated version of vtt.js, as there is a small bug in the master. I forked it and created a pull request, and you can find a fixed version of vtt.js here: https://github.com/mediathand/vtt.js/tree/VTTCue-fix

I added a reference to the vtt.js file to the demo index.html file just for testing. Maybe that's not the best solution, but I'll leave dealing with this to the main contributors. :)

kanongil and others added 15 commits September 27, 2016 12:52
Added WebVTT option to config (on by default - switched CEA708 off by default).
Added tracks to player for subtitle languages on manifest loaded.
Extended playlist loader to handle subtitle tracks.
Modified playlist loader to identify subtitle playlists as such.
Initial work on subtitle stream controller.
Activated parsing and downloading of subtitle fragment by triggering event.
Initial work on WebVTT file content parser.
Added events, generic WebVTT error.
Added event and logic to extract initial PTS via event for subtitle synchronisation.
Added 33-bit wraparound fix.
Fixed encoding of special characters.
Added event fired when subtitle fragment has been processed and a new can be fetched.
Store unparsed fragments if no initial PTS has been received.
Add cues to media when parsed.
Fire event upon parsing success or failure.
Reload live playlist at interval (10 seconds).
Created queue for subtitle fragments awaiting fetching and parsing.
Keep track of successfully processed fragments.
Set subtitles labeled default to "showing".
Add subtitle track ID to fragment and got rid of silly guesswork.
Cleaned a bit of code.
Also triggered event in case no data is received, for whatever reason.
Fixed typo.
…iable:

"{fooBar : fooBar}" is the same as "{fooBar}"
@@ -90,6 +114,25 @@ class TimelineController extends EventHandler {
this.lastPts = Number.NEGATIVE_INFINITY;
}

onManifestLoaded(data) {
// TODO: actually remove the tracks from the media object.
Copy link
Collaborator

Choose a reason for hiding this comment

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

It's not possible to remove tracks from a video tag - only cues from the tracks object

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Hah, yeah I found out some time after writing that comment. Then forgot to remove it. Sorry about that. 😀

}

/** get index of the selected subtitle track (index in subtitle track lists) **/
get subtitleTrack() {
Copy link
Collaborator

Choose a reason for hiding this comment

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

I think this is better worded as currentTrack, following the trend of levels and currentLevel

@johnBartos
Copy link
Collaborator

@coolheinze I'd really like this in before Chrome 55 so I'll review and test this PR on my end - @mangui Have you had a chance to look at this yet? Happy to help here.

@@ -0,0 +1,145 @@
/*
* audio track controller
Copy link
Collaborator

Choose a reason for hiding this comment

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

subtitle

}

onMediaDetaching() {
// TODO: Remove event listeners.
Copy link
Collaborator

Choose a reason for hiding this comment

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

this.unregisterListeners() should do it

@coolheinze
Copy link
Contributor Author

@johnBartos Keep in mind, though, that vtt.js (https://github.com/mozilla/vtt.js) has a bug (tiny, easy to fix but critical nonetheless). I made a pull request a long time ago, which they haven't looked at, so they won't fix it in a hurry, by the looks of it. But I'm sure you'll manage quite well. 😄

@johnBartos
Copy link
Collaborator

@coolheinze mediathand#1

@coolheinze
Copy link
Contributor Author

@johnBartos seems vtt.js have approved my PR: mozilla/vtt.js#353

@johnBartos
Copy link
Collaborator

@coolheinze I had a PR up which had this PR in a working state but there were some issues with it (so I took it down). Just some stuff from our fork that shouldn't be in the PR. However, there's one real issue: removing text tracks. Since we cannot remove them duplicates or extraneous tracks become a serious problem when switching streams or coming back from an ad break.

Say you switch from stream 1 to stream 2, where stream 1 had tracks and stream 2 has new tracks
What happens here is that no new cues are added to stream 2's tracks since your ID mechanism always starts counting at 0 (i.e. it assumes that there are no tracks already existing). Another case is when you switch from stream 1 to stream 2 when stream 2 has no tracks - the video tag still does, however.

I tried reusing the tracks but ran into several problems there; however, coming up with a reuse system is definitely the way to go. In our player, we remove and replace the video tag whenever we switch streams to avoid this problem.

@coolheinze
Copy link
Contributor Author

Yeah, I am well aware of that problem. Quite odd that you cannot remove the tracks, but it might be because they can be added through several mechanisms and not just programmatically.

Anyway, that's another story.

We would - in our use case - tear down and replace the video tag like you wrote. That is, however, up to the script implementing hls.js to handle that, so we left it where it is.

I haven't encountered your mid roll ad issue, as it is pretty far from our use case. However, my take on it would be to create another video tag on top and keep the ad and main clip separated hermetically.

@mangui
Copy link
Member

mangui commented Jan 13, 2017

@mangui mangui closed this Jan 13, 2017
@mangui
Copy link
Member

mangui commented Jan 13, 2017

@coolheinze
Copy link
Contributor Author

coolheinze commented Jan 13, 2017

My pleasure! 😄

If there's anything else you need help with, don't expect me to be useful at all! 😆

@mangui
Copy link
Member

mangui commented Jan 13, 2017

@coolheinze
Copy link
Contributor Author

Disney must be turning in his grave... 😆

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants