Peer5 / videojs-contrib-hls.js

HLS library for video.js using Dailymotion's hls.js tech
Apache License 2.0
108 stars 55 forks source link

Fix caption integration #25

Closed tmm1 closed 7 years ago

tmm1 commented 7 years ago

This is a rebased version of #19 with additional fixes.

cc @ranuser99 @willkelleher

tmm1 commented 7 years ago

With this change, I now see a CC button in Chrome with an english option listed. Picking the option renders the captions, however when there are multiple lines they appear upside down.

tmm1 commented 7 years ago

I can't figure out why the cues are showing up in the wrong order..

This hacky change to video.js fixes it:

diff --git a/src/js/tracks/text-track-display.js b/src/js/tracks/text-track-display.js
index 0097dd67..afd3fce3 100644
--- a/src/js/tracks/text-track-display.js
+++ b/src/js/tracks/text-track-display.js
@@ -242,6 +242,7 @@ class TextTrackDisplay extends Component {
     for (let i = 0; i < track.activeCues.length; i++) {
       cues.push(track.activeCues[i]);
     }
+    cues.reverse();

     window.WebVTT.processCues(window, cues, this.el_);
mrbar42 commented 7 years ago

Couldn't reproduce so far, neither the missing button nor the reverse order. @tmm1 on what browsers are you able to reproduce?

tmm1 commented 7 years ago

@mrbar42 this issue is specific to a/53 captions (also known CEA608/708 or EIA608) which are embedded in live tv video packets.

See @ranuser99's example stream: https://1-fss-fso35.streamhoster.com/lv_ntv/broadcast1/playlist.m3u8

tmm1 commented 7 years ago

Here's the stream that hls.js uses in their tests: http://playertest.longtailvideo.com/adaptive/captions/playlist.m3u8

You can try it on http://video-dev.github.io/hls.js/demo/?src=http%3A%2F%2Fplayertest.longtailvideo.com%2Fadaptive%2Fcaptions%2Fplaylist.m3u8&enableStreaming=true&autoRecoverError=true&enableWorker=true&dumpfMP4=false&levelCapping=-1&defaultAudioCodec=undefined and see that captions are rendered. The same test with videojs on chrome should show the problem.

mrbar42 commented 7 years ago

My testing of this: VideoJS <= 5.13.1 on Firefox 53 where text tracks has no native support, this PR fixes the problem on Chrome 58 where text tracks are natively supported, the button does not appear.

VideoJS > 5.13.1 Both Firefox and Chrome doesn't show the button like described in the issue.

it looks like the problem is that video.js is unaware of the captions. when setting videoTag.controls = true the captions button is shown on the native controls.

seems like another fix is needed for newer videojs versions (not in the scope of this PR)

willkelleher commented 7 years ago

Thanks @tmm1 !