Support native captions in html5 tech. #1656

gkatsev · 2014-11-11T19:55:56Z

Don't auto-add the textTrackDisplay component.
Add a 'featuresNativeTrack'. Currently hardcoded to false for everything
except the html5 tech.
Add textTracks and addTextTrack methods to html5 tech.
Proxy player level methods to tech if we're using native tracks.
Fix up track menu items to work with spec tracks or vjs tracks.

This is the beginning of step 1 for the revamp of text tracks.

Don't auto-add the textTrackDisplay component. Add a 'featuresNativeTrack'. Currently hardcoded to false for everything except the html5 tech. Add textTracks and addTextTrack methods to html5 tech. Proxy player level methods to tech if we're using native tracks. Fix up track menu items to work with spec tracks or vjs tracks.

gkatsev · 2014-11-11T23:36:18Z

Looks like firefox doesn't like to show captions.

heff · 2014-11-12T00:32:32Z

It doesn't show captions at all?

heff · 2014-11-12T00:33:36Z

src/js/media/html5.js

@@ -27,6 +27,8 @@ vjs.Html5 = vjs.MediaTechController.extend({
    // HTML video supports progress events
    this['featuresProgressEvents'] = true;

+    this['featuresNativeTracks'] = true;


we should probably use textTrack instead of just tracks here, because of videoTracks and audioTracks

gkatsev · 2014-11-12T00:35:50Z

Firefox seems to be complicated but you can force it to show captions natively.

heff · 2014-11-12T00:38:54Z

src/js/media/html5.js

@@ -98,6 +100,20 @@ vjs.Html5.prototype.createEl = function(){
    // associate the player with the new tag
    el['player'] = player;

+    if (player.options_.tracks) {


Right now the player strips and disables all the tracks from the video tag at init, and this this would add them back in. It might still be the right way to do since tracks could be overridden by other options, but any thoughts on that?

I've disabled the stripping of tracks. This is to re-add them because we re-create the element here on iOS devices or other devices that have the movingMediaElementInDOM flag.

gkatsev · 2014-11-12T20:49:11Z

Oh, I think firefox is being weird in a way related to iOS. Re-adding the track elements fixes the issues.

gkatsev · 2014-11-12T22:06:06Z

I think the issue is that we're moving the video element and its track elements before firefox has a change to deal with them and it never gets around to fetching the tracks again. I added a fix to kick firefox into gear to re-fetch the tracks but I did it too late in the process so textTracks still returns nothing until the tracks get loaded.

gkatsev · 2014-11-12T22:13:51Z

I'm leaning towards not supporting firefox natively just yet.

gkatsev · 2014-11-12T23:18:31Z

Looks like HTC's native android browsers don't support native text tracks.
Older samsung devices seem to support native text tracks but for some reason vjs's custom implementation is showing up on older versions of android.
It's possible we may want to just always have custom captions on android browsers if they are not Chrome.

heff · 2014-11-13T00:48:47Z

src/js/player.js

@@ -227,6 +227,9 @@ vjs.Player.prototype.createEl = function(){
  // Remove width/height attrs from tag so CSS can make it 100% width/height
  tag.removeAttribute('width');
  tag.removeAttribute('height');
+
+  /* Don't clear out tracks


These comments don't make for a very nice diff. :)

Nope, I'll clear it out completely during cleanup.

gkatsev · 2014-11-13T22:42:10Z

Some native android browsers are old and use numerical mode values. Trying to mess with the value to switch tracks causes the track to be hidden and you cannot re-display it. Devices with numerical modes for text-tracks should be considered off the table for native text track capabilities.

Also, if text tracks are old and use numerical mode values, use custom tracks. If an html tech is using custom tracks, remove track elements so we don't accidentally show both custom and native captions. Clean up showTextTrack slightly.

heff · 2014-11-13T23:57:35Z

Unfortunate but reasonable. Which android versions/browsers are we talking about?

gkatsev · 2014-11-14T00:01:51Z

GS4's Native Android Browser. It's running Android 4.2.2 and I think it's native browser may have been based on Chrome 18.

gkatsev · 2014-11-14T19:38:47Z

IE11 (and maybe 10) will not interpret a mimetype of application/octet-stream as text/vtt if the file is named something like foo.vtt.

gkatsev · 2014-11-14T20:03:39Z

Looks like IE10 also uses numeric modes and has the same issue as the older android devices that do the same. As soon as we try to change the mode, the captions disappear.

This brings me to the following though: if text tracks are supported but have weird issues like the mode, do we want, in those cases, let the browser do it's best to pick the right caption and hide our captions button, or just have a button that disabled captions? @heff @dmlap. Firefox and Chrome will choose the "best" option to display and the display it. Chrome gives the users a button to toggle those captions on or off while Firefox does not.

gkatsev · 2014-11-14T20:08:59Z

IE8 and IE9 don't support native captions, so, the custom implementation is being used.

gkatsev · 2014-11-14T20:28:48Z

I cannot figure out how to get captions working in Firefox... @RickEyre would you be able to provide some direction for that?

gkatsev · 2014-11-17T17:05:08Z

Decided to punt on Firefox for now.

heff · 2014-11-19T19:35:34Z

src/js/media/media.js

+ * Html5 tech overrides these.
+ */
+vjs.MediaTechController.prototype.textTracks = function() {
+  this.player_.textTracks_ = this.player_.textTracks_ || [];


Do we still need to store the tracks on the player or can we move them to tech.textTracks_?

Um... I was going to say that it was because I didn't want to lose the tracks if the tech switches. But the tracks are available in the player options anyway, so, may not be necessary. I'll take a look to see whether we need to keep it on the player now as well.

Yeah, I was wondering about the tech switching case. Techs at least won't switch unless the source is changed, so the question is should tracks persist after a new source is set. I'm not sure how the video element handles that but I would be tempted to say that no matter what the video element does, if you set a new source you have to provide new tracks for the source at the same time. If tracks (including dynamically created ones) need to persist then that would create a much bigger headache for us as far as translating native tracks to non-native tracks in a tech switch.

heff · 2014-11-19T20:03:58Z

Very nice work, man. Between the native support and what's in video.js today, I know this is a lot to wrap your head around. There's still some outstanding issues around fully shimming things, and it's not always clear whether some of that should be done now or after the vtt.js research.

heff · 2014-11-19T21:03:18Z

src/js/tracks.js

+        return;
+      }
+
+      controlBar.getChild('subtitlesButton').update();


After you add the change/add/remove listeners to the buttons these shouldn't be needed anymore.

Does our custom textTracks fire those events?

Since this piece of code is strictly for the custom controls.

Not now but I think in the end result it should.

Yes, it should. But that would come as part of the next steps.

i.e. I don't think you need to wait for the next steps to update how these buttons work. They should expect native tracks or some that looks exactly like them.

showTextTrack has been updated to use the correct mode for native text tracks.
I'd prefer to update the buttons in another PR to not make this one any bigger.

Ah, ok. So the goal for this PR is to get the native tracks API working, while maintaining the custom tracks support without getting deeper into shimming. And get this merged in before taking a second pass at shimming.

Yes, exactly.

Which requires the getProp function in the transition, which if that's it I guess that's not terrible. I'm not totally sure if we should release this without the second piece. Technically it breaks the track object API when native is used, so half the time, at least until the second pass when we break it fully. Know any players that have a custom captions button?

gkatsev · 2014-11-19T22:39:20Z

@heff updated based on code review comments

MenuButton's new 'update' method conflicted with volumeMenuButton's update method which was used to update volume levels.

heff · 2014-11-25T21:06:12Z

test/unit/media.js

+    oldTextTracks = videojs.MediaTechController.prototype.textTracks;
+    videojs.MediaTechController.prototype.textTracks = function() {
+      return {
+        addEventListener: Function.prototype


Function.prototype is a cool trick but may not be super obvious for the next person who has to debug this.

heff · 2014-11-25T21:09:32Z

test/unit/tracks.js

+  vjs.Player.prototype.addTextTrack = function() {
+    add++;
+  };
+  vjs.Player.prototype.getChild = function(child) {


I think this deserves a comment to explain what's happening here.

gkatsev · 2014-12-03T22:24:27Z

This is good to go but we've decided to merge all caption stuff together. closing for now, especially since it now needs to be rebased against master.

gkatsev mentioned this pull request Nov 11, 2014

Revamped TextTrack functionality #1655

Closed

17 tasks

Re-add text tracks on 'movingMediaElementInDOM'

300161e

heff reviewed Nov 12, 2014
View reviewed changes

heff reviewed Nov 13, 2014
View reviewed changes

Use custom tracks if no native textTracks support

b98edfd

Also, if text tracks are old and use numerical mode values, use custom tracks. If an html tech is using custom tracks, remove track elements so we don't accidentally show both custom and native captions. Clean up showTextTrack slightly.

gkatsev mentioned this pull request Nov 14, 2014

iPhone overrides videojs player and captions #1653

Closed

Use setAttribute and 'in' so IE8 doesn't feak out.

f4976f7

dispose of tracks properly

85249f6

gkatsev mentioned this pull request Nov 17, 2014

Serious Track Issue - Video.js Cannot Support Dynamically Change/Load Track File #1664

Closed

Firefox gets custom captions for now

c16d96e

gkatsev mentioned this pull request Nov 17, 2014

Better timeout handling #1642

Closed

heff reviewed Nov 19, 2014
View reviewed changes

gkatsev added 3 commits November 19, 2014 15:51

have vjs do the "addChild" "magic"

13bb2df

Rename featuresNativeTracks to featuresTextTracks

6a4b3fd

Move textTracks_ into tech

043c211

heff reviewed Nov 19, 2014
View reviewed changes

gkatsev added 2 commits November 19, 2014 16:19

Update text track btns based on add/remove events

58ff9ae

Update tests

6529faa

Rename volumeMenuButton's update to volumeUpdate

ca3cffc

MenuButton's new 'update' method conflicted with volumeMenuButton's update method which was used to update volume levels.

gkatsev mentioned this pull request Nov 25, 2014

Captions #1691

Closed

heff reviewed Nov 25, 2014
View reviewed changes

gkatsev added 2 commits November 25, 2014 16:07

Add a text track to test video

3ed3f26

Update html5 tech to use test_vid tracks

f39c0af

heff reviewed Nov 25, 2014
View reviewed changes

gkatsev added 4 commits November 25, 2014 16:15

Add a number mode to test

0b2e856

Get 'mode' as a string so gcc won't squash it

f1ed990

Add comment, remove extra vertical whitespace

14f5748

Remove extra whitespace and unused var

dd911a5

mmcc mentioned this pull request Dec 1, 2014

vjs.hasData Error Spam #1484

Closed

heff added confirmed enhancement labels Dec 2, 2014

gkatsev closed this Dec 3, 2014

gkatsev deleted the native-captions branch March 8, 2016 20:39

Support native captions in html5 tech. #1656

Support native captions in html5 tech. #1656

Conversation

gkatsev commented Nov 11, 2014

gkatsev commented Nov 11, 2014

heff commented Nov 12, 2014

Choose a reason for hiding this comment

gkatsev commented Nov 12, 2014

Choose a reason for hiding this comment

Choose a reason for hiding this comment

gkatsev commented Nov 12, 2014

gkatsev commented Nov 12, 2014

gkatsev commented Nov 12, 2014

gkatsev commented Nov 12, 2014

Choose a reason for hiding this comment

Choose a reason for hiding this comment

gkatsev commented Nov 13, 2014

heff commented Nov 13, 2014

gkatsev commented Nov 14, 2014

gkatsev commented Nov 14, 2014

gkatsev commented Nov 14, 2014

gkatsev commented Nov 14, 2014

gkatsev commented Nov 14, 2014

gkatsev commented Nov 17, 2014

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

heff commented Nov 19, 2014

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

gkatsev commented Nov 19, 2014

Choose a reason for hiding this comment

Choose a reason for hiding this comment

gkatsev commented Dec 3, 2014