Add support for subtitles on Chromecast #1165
Conversation
|
any update on this? i love the player but i usually cast my videos and not having subtitles is kind of a deal breaker, so this would be awesome |
src/renderer/lib/cast.js
Outdated
| cb() | ||
| } else { | ||
| ret.subServer = http.createServer((req, res) => { | ||
| if (!selectedSubtitle) { |
There was a problem hiding this comment.
I don't think this code will ever be reached because selectedSubtile has already been checked for a value in the above if clause.
src/renderer/lib/cast.js
Outdated
| @@ -130,22 +132,51 @@ function chromecastPlayer () { | |||
| }) | |||
| } | |||
|
|
|||
| function serveSubtitles (cb) { | |||
There was a problem hiding this comment.
In all functions in this file callback arguments are called callback, probably best to keep this consistent.
src/renderer/lib/cast.js
Outdated
| state.playing.location = 'chromecast' | ||
| } | ||
| update() | ||
| serveSubtitles(function (subtitles) { |
There was a problem hiding this comment.
Since the subtitles variable is not actually an array of subtitles it makes more sense to use the same name as used before in the serveSubtitles function - namely subtitlesUrl.
janza
force-pushed
the
chromecast-subtitles
branch
from
171c1ab
to
f7ec537
Compare
|
Hi all! Would be amazing to have this PR merged!!! |
|
This looks like a useful PR. Will find a Chromecast to test it on as soon as I can |
|
Any update on this? |
|
Hey there, I built the master branch and cherry-picked janza@2508701 and it works! |
|
Just did the same here like @gonzaloserrano did, and it worked for me too! Nice job @janza! @feross Any change to merge this PR? |
|
I've tested this PR and it seems to work on both my Phillips TV's built-in Chromecast, a Chromecast Gen. 1, and the Chromecast Ultra without any problems. |
Support subtitles on Chromecast by creating a new http server for selected subtitles before calling player.play()
Closes #530