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

Fix caption integration #19

Closed
wants to merge 1 commit into from
Closed

Fix caption integration #19

wants to merge 1 commit into from

Conversation

willkelleher
Copy link
Contributor

This may be a little hacky, but it does fix in-band caption integration on versions of video.js > 5.13.0.

Related to #17

@ranuser99
Copy link

ranuser99 commented May 4, 2017

There are several drawing/display issues with live streams. It's very hard to visually follow.

Here's an example captioned live stream:

https://1-fss-fso35.streamhoster.com/lv_ntv/broadcast1/playlist.m3u8

The hls.js demo displays this perfectly (new lines on bottom and grow as new text arrives, pushing old content up):

http://streambox.fr/mse/hls.js-0.7.6/demo/

Is hls.js leveraging native caption drawing in the browser? How can videojs take advantage of native caption functionality when available?

@tmm1
Copy link
Contributor

tmm1 commented May 5, 2017

@ranuser99 Did you try reverting videojs/video.js#3798, as it seems the problems started after that commit

writable: false
});
el.addTextTrack = function() {
return tech.addTextTrack(arguments);
Copy link
Contributor

Choose a reason for hiding this comment

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

This is incorrect, as it passes the array of arguments in as a single argument. It should read:

return tech.addTextTrack.apply(tech, arguments)

@tmm1 tmm1 mentioned this pull request May 5, 2017
@ranuser99
Copy link

The issue is apparently solved using older videojs 5.13.2 (from another user's suggestion).

@mrbar42
Copy link
Collaborator

mrbar42 commented May 8, 2017

superseded by #25
thanks for the PR

@mrbar42 mrbar42 closed this May 8, 2017
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.

4 participants