From 3a4906079bed4050396d78982963128ca511d1f3 Mon Sep 17 00:00:00 2001 From: Mathias Rasmussen Date: Tue, 13 Sep 2016 02:46:48 +0200 Subject: [PATCH] External player clean up (#914) * minor `addSubtitles` clean up * external player clean up --- src/main/external-player.js | 20 ++++++++++++------- .../components/unsupported-media-modal.js | 7 +------ .../controllers/subtitles-controller.js | 13 ++++++------ src/renderer/lib/state.js | 9 ++++++++- src/renderer/pages/player-page.js | 6 +----- src/renderer/pages/preferences-page.js | 15 +++++--------- 6 files changed, 34 insertions(+), 36 deletions(-) diff --git a/src/main/external-player.js b/src/main/external-player.js index a048a082..60da7b0b 100644 --- a/src/main/external-player.js +++ b/src/main/external-player.js @@ -5,6 +5,7 @@ module.exports = { } const cp = require('child_process') +const path = require('path') const vlcCommand = require('vlc-command') const log = require('./log') @@ -13,15 +14,15 @@ const windows = require('./windows') // holds a ChildProcess while we're playing a video in an external player, null otherwise let proc = null -function checkInstall (path, cb) { +function checkInstall (playerPath, cb) { // check for VLC if external player has not been specified by the user // otherwise assume the player is installed - if (path == null) return vlcCommand((err) => cb(!err)) + if (playerPath == null) return vlcCommand((err) => cb(!err)) process.nextTick(() => cb(true)) } -function spawn (path, url, title) { - if (path != null) return spawnExternal(path, [url]) +function spawn (playerPath, url, title) { + if (playerPath != null) return spawnExternal(playerPath, [url]) // Try to find and use VLC if external player is not specified vlcCommand(function (err, vlcPath) { @@ -44,10 +45,15 @@ function kill () { proc = null } -function spawnExternal (path, args) { - log('Running external media player:', path + ' ' + args.join(' ')) +function spawnExternal (playerPath, args) { + log('Running external media player:', playerPath + ' ' + args.join(' ')) - proc = cp.spawn(path, args, {stdio: 'ignore'}) + if (path.extname(playerPath) === '.app') { + // Mac: Use executable in packaged .app bundle + playerPath += '/Contents/MacOS/' + path.basename(playerPath, '.app') + } + + proc = cp.spawn(playerPath, args, {stdio: 'ignore'}) // If it works, close the modal after a second const closeModalTimeout = setTimeout(() => diff --git a/src/renderer/components/unsupported-media-modal.js b/src/renderer/components/unsupported-media-modal.js index f2c29580..70920b97 100644 --- a/src/renderer/components/unsupported-media-modal.js +++ b/src/renderer/components/unsupported-media-modal.js @@ -1,6 +1,5 @@ const React = require('react') const electron = require('electron') -const path = require('path') const ModalOKCancel = require('./modal-ok-cancel') const {dispatcher} = require('../lib/dispatcher') @@ -12,15 +11,11 @@ module.exports = class UnsupportedMediaModal extends React.Component { const message = (err && err.getMessage) ? err.getMessage() : err - const playerPath = state.saved.prefs.externalPlayerPath - const playerName = playerPath - ? path.basename(playerPath).split('.')[0] - : 'VLC' const onAction = state.modal.externalPlayerInstalled ? dispatcher('openExternalPlayer') : () => this.onInstall() const actionText = state.modal.externalPlayerInstalled - ? 'PLAY IN ' + playerName.toUpperCase() + ? 'PLAY IN ' + state.getExternalPlayerName().toUpperCase() : 'INSTALL VLC' const errorMessage = state.modal.externalPlayerNotFound ? 'Couldn\'t run external player. Please make sure it\'s installed.' diff --git a/src/renderer/controllers/subtitles-controller.js b/src/renderer/controllers/subtitles-controller.js index 727dc040..04c4797f 100644 --- a/src/renderer/controllers/subtitles-controller.js +++ b/src/renderer/controllers/subtitles-controller.js @@ -33,11 +33,10 @@ module.exports = class SubtitlesController { } addSubtitles (files, autoSelect) { - const state = this.state // Subtitles are only supported when playing video files - if (state.playing.type !== 'video') return + if (this.state.playing.type !== 'video') return if (files.length === 0) return - const subtitles = state.playing.subtitles + const subtitles = this.state.playing.subtitles // Read the files concurrently, then add all resulting subtitle tracks const tasks = files.map((file) => (cb) => loadSubtitle(file, cb)) @@ -47,17 +46,17 @@ module.exports = class SubtitlesController { for (let i = 0; i < tracks.length; i++) { // No dupes allowed const track = tracks[i] - let trackIndex = state.playing.subtitles.tracks - .findIndex((t) => track.filePath === t.filePath) + let trackIndex = subtitles.tracks.findIndex((t) => + track.filePath === t.filePath) // Add the track if (trackIndex === -1) { - trackIndex = state.playing.subtitles.tracks.push(track) - 1 + trackIndex = subtitles.tracks.push(track) - 1 } // If we're auto-selecting a track, try to find one in the user's language if (autoSelect && (i === 0 || isSystemLanguage(track.language))) { - state.playing.subtitles.selectedIndex = trackIndex + subtitles.selectedIndex = trackIndex } } diff --git a/src/renderer/lib/state.js b/src/renderer/lib/state.js index 2e70f296..f2734c4d 100644 --- a/src/renderer/lib/state.js +++ b/src/renderer/lib/state.js @@ -68,7 +68,8 @@ function getDefaultState () { * Getters, for convenience */ getPlayingTorrentSummary, - getPlayingFileSummary + getPlayingFileSummary, + getExternalPlayerName } } @@ -168,6 +169,12 @@ function getPlayingFileSummary () { return torrentSummary.files[this.playing.fileIndex] } +function getExternalPlayerName () { + const playerPath = this.saved.prefs.externalPlayerPath + if (!playerPath) return 'VLC' + return path.basename(playerPath).split('.')[0] +} + function load (cb) { const state = getDefaultState() diff --git a/src/renderer/pages/player-page.js b/src/renderer/pages/player-page.js index 0f67f0c9..9bd0b1d5 100644 --- a/src/renderer/pages/player-page.js +++ b/src/renderer/pages/player-page.js @@ -2,7 +2,6 @@ const React = require('react') const Bitfield = require('bitfield') const prettyBytes = require('prettier-bytes') const zeroFill = require('zero-fill') -const path = require('path') const TorrentSummary = require('../lib/torrent-summary') const Playlist = require('../lib/playlist') @@ -289,11 +288,8 @@ function renderCastScreen (state) { castType = 'DLNA' isCast = true } else if (state.playing.location === 'external') { - // TODO: get the player name in a more reliable way - const playerPath = state.saved.prefs.externalPlayerPath - const playerName = playerPath ? path.basename(playerPath).split('.')[0] : 'VLC' castIcon = 'tv' - castType = playerName + castType = state.getExternalPlayerName() isCast = false } else if (state.playing.location === 'error') { castIcon = 'error_outline' diff --git a/src/renderer/pages/preferences-page.js b/src/renderer/pages/preferences-page.js index 28b2fcf5..018aa08f 100644 --- a/src/renderer/pages/preferences-page.js +++ b/src/renderer/pages/preferences-page.js @@ -1,11 +1,11 @@ -const colors = require('material-ui/styles/colors') const path = require('path') const React = require('react') +const colors = require('material-ui/styles/colors') const Checkbox = require('material-ui/Checkbox').default +const RaisedButton = require('material-ui/RaisedButton').default const Heading = require('../components/heading') const PathSelector = require('../components/path-selector') -const RaisedButton = require('material-ui/RaisedButton').default const {dispatch} = require('../lib/dispatcher') @@ -59,9 +59,8 @@ class PreferencesPage extends React.Component { } externalPlayerPathSelector () { - const playerName = path.basename( - this.props.state.unsaved.prefs.externalPlayerPath || 'VLC' - ) + const playerPath = this.props.state.unsaved.prefs.externalPlayerPath + const playerName = this.props.state.getExternalPlayerName() const description = this.props.state.unsaved.prefs.openExternalPlayer ? `Torrent media files will always play in ${playerName}.` @@ -79,16 +78,12 @@ class PreferencesPage extends React.Component { displayValue={playerName} onChange={this.handleExternalPlayerPathChange} title='External player' - value={this.props.state.unsaved.prefs.externalPlayerPath} /> + value={playerPath ? path.dirname(playerPath) : null} /> ) } handleExternalPlayerPathChange (filePath) { - if (path.extname(filePath) === '.app') { - // Mac: Use executable in packaged .app bundle - filePath += '/Contents/MacOS/' + path.basename(filePath, '.app') - } dispatch('updatePreferences', 'externalPlayerPath', filePath) }