From fa41e5aae1c6874e730e6aff818f3ff57ba6d68e Mon Sep 17 00:00:00 2001 From: Florrie Date: Mon, 18 Sep 2017 17:20:59 -0300 Subject: Track-specific converter options See todo.txt, changes in the man page, and this example: https://gist.github.com/towerofnix/9b1853ee74d30357e1b59d22c59aba2f --- man/http-music-play.1 | 18 ++++++++++++ src/downloaders.js | 45 +++++++++++++++++++----------- src/loop-play.js | 34 +++++++++++++++-------- src/pickers.js | 6 ++-- src/play.js | 77 +++++++++++++++++++++++++++++++++++++++++++++++++-- todo.txt | 21 ++++++++++++++ 6 files changed, 169 insertions(+), 32 deletions(-) diff --git a/man/http-music-play.1 b/man/http-music-play.1 index 7da8ca8..0a82246 100644 --- a/man/http-music-play.1 +++ b/man/http-music-play.1 @@ -79,10 +79,28 @@ Skips past the track that's currently playing. Clears the active playlist. This does not effect the source playlist, so specific groups can be selected using \fB\-\-keep\fR. +.TP +.BR \-\-converter +Sets the program used for converting tracks. +By default the program is either \fBffmpeg\fR or \fBavconv\fR. +Playlists which use track-specific converter options should use this option through the \fB"options"\fR playlist property to set an intended converter program. +If the program specified through \-\-converter does not exist, converter options will not be used (see \fB\-\-enable\-converter\-options\fR). + +.TP +.BR \-\-disable\-converter\-options ", " \-\-no\-use\-converter\-options +Forces track-specific converter options to not be used. +See also \fB\-\-enable\-converter\-options\fR. + .TP .BR \-\-disable\-playback\-status ", " \-\-hide\-playback\-status Hides playback status (timestamps, etc). +.TP +.BR \-\-enable\-converter\-options ", " \-\-use\-converter\-options +Forces usage of track-specific converter options. +By default, they are enabled. +See also \fB\-\-disable\-converter\-options\fR. + .TP .BR \-h ", " \-? ", " \-\-help Presents a help message, directing the user to the \fBman\fR page. diff --git a/src/downloaders.js b/src/downloaders.js index c41efa5..138b2d6 100644 --- a/src/downloaders.js +++ b/src/downloaders.js @@ -110,26 +110,39 @@ function makePowerfulDownloader(downloader, maxAttempts = 5) { } } -async function makeConverter(type) { - let binary - if (await commandExists('avconv')) { - binary = 'avconv' - } else if (await commandExists('ffmpeg')) { - binary = 'ffmpeg' - } else { - throw new Error('avconv or ffmpeg is required for converter downloader!') +async function makeConverter( + converterCommand = null, exportExtension = 'wav', + converterOptions = ['-i', '$in', '$out'] +) { + if (converterCommand === null) { + throw new Error( + 'A converter is required! Try installing ffmpeg or avconv?' + ) } - console.log(`Using ${binary} converter.`) - - return async function(inFile) { - const base = path.basename(inFile, path.extname(inFile)) - const tempDir = tempy.directory() - const outFile = `${tempDir}/${base}.${type}` + return function(converterOptions = ['-i', '$in', '$out']) { + return async function(inFile) { + const base = path.basename(inFile, path.extname(inFile)) + const tempDir = tempy.directory() + const outFile = `${tempDir}/${base}.${exportExtension}` + + const processedOptions = converterOptions.slice(0) + + // And some people say JavaScript isn't awesome!? + for (const [ i, item ] of processedOptions.entries()) { + if (item === '$in') { + processedOptions[i] = inFile + } else if (item === '$out') { + processedOptions[i] = outFile + } + } - await promisifyProcess(spawn(binary, ['-i', inFile, outFile]), false) + await promisifyProcess( + spawn(converterCommand, processedOptions), false + ) - return outFile + return outFile + } } } diff --git a/src/loop-play.js b/src/loop-play.js index 72e770f..d22833f 100644 --- a/src/loop-play.js +++ b/src/loop-play.js @@ -183,14 +183,14 @@ class SoXPlayer extends Player { } class DownloadController extends EventEmitter { - constructor(playlist) { + constructor(playlist, converterProgram) { super() - this.playlist = playlist + Object.assign(this, {playlist, converterProgram}) } async init() { - this.converter = await makeConverter('wav') + this.converter = await makeConverter(this.converterProgram) } waitForDownload() { @@ -215,7 +215,7 @@ class DownloadController extends EventEmitter { }) } - async download(downloader, arg) { + async download(downloader, downloaderArg, converterOptions) { // Downloads a file. This doesn't return anything; use // waitForDownload to get the result of this. // (The reasoning is that it's possible for a download to @@ -240,7 +240,7 @@ class DownloadController extends EventEmitter { // 'convertErrored'. try { - downloadFile = await downloader(arg) + downloadFile = await downloader(downloaderArg) } catch(err) { this.emit('errored', err) return @@ -256,7 +256,7 @@ class DownloadController extends EventEmitter { let convertFile try { - convertFile = await this.converter(downloadFile) + convertFile = await this.converter(converterOptions)(downloadFile) } catch(err) { this.emit('errored', err) return @@ -300,6 +300,7 @@ class PlayController extends EventEmitter { this.playlist = playlist this.historyController = historyController this.downloadController = downloadController + this.useConverterOptions = true this.currentTrack = null this.nextTrack = null @@ -387,7 +388,14 @@ class PlayController extends EventEmitter { downloader = getDownloaderFor(picked.downloaderArg) } - this.downloadController.download(downloader, picked.downloaderArg) + if (picked.converterOptions && !Array.isArray(picked.converterOptions)) { + throw new Error("The converterOptions track property must be an array") + } + + this.downloadController.download( + downloader, picked.downloaderArg, + this.useConverterOptions ? picked.converterOptions : undefined + ) this.downloadController.waitForDownload() .then(file => { @@ -395,12 +403,13 @@ class PlayController extends EventEmitter { this.nextFile = file this.emit('downloaded') }) - .catch(() => { + .catch(err => { // TODO: Test this!! console.warn( "\x1b[31mFailed to download (or convert) track \x1b[1m" + getItemPathString(this.nextTrack) + "\x1b[0m" ) + console.warn(err) // A little bit blecht, but.. this works. // "When a track fails, remove it from the timeline, and start @@ -509,7 +518,8 @@ class PlayController extends EventEmitter { module.exports = async function startLoopPlay( playlist, { - pickerOptions, playerCommand = 'mpv', + pickerOptions, playerCommand, converterCommand, + useConverterOptions = true, disablePlaybackStatus = false } ) { @@ -540,7 +550,9 @@ module.exports = async function startLoopPlay( Object.assign(player, {disablePlaybackStatus}) - const downloadController = new DownloadController(playlist) + const downloadController = new DownloadController( + playlist, converterCommand + ) await downloadController.init() const historyController = new HistoryController( @@ -551,7 +563,7 @@ module.exports = async function startLoopPlay( player, playlist, historyController, downloadController ) - Object.assign(playController, {playerCommand}) + Object.assign(playController, {playerCommand, useConverterOptions}) const promise = playController.loopPlay() diff --git a/src/pickers.js b/src/pickers.js index 9f1cd40..245e16f 100644 --- a/src/pickers.js +++ b/src/pickers.js @@ -192,7 +192,8 @@ function generalPicker(sourcePlaylist, lastTrack, options) { if (options.hasOwnProperty(playlistCache)) { playlist = options[playlistCache] } else { - console.log('\x1b[1K\rIndexing (flattening)...') + // TODO: Enable this conditionally. + // console.log('\x1b[1K\rIndexing (flattening)...') if (typeof options.seed === 'undefined') { options.seed = Math.random() @@ -207,7 +208,8 @@ function generalPicker(sourcePlaylist, lastTrack, options) { options[playlistCache] = playlist - console.log('\x1b[1K\rDone indexing.') + // TODO: Enable this condtionally. + // console.log('\x1b[1K\rDone indexing.') } const index = playlist.items.indexOf(lastTrack) diff --git a/src/play.js b/src/play.js index 0fdebc7..c27d2f3 100755 --- a/src/play.js +++ b/src/play.js @@ -55,6 +55,16 @@ async function determineDefaultPlayer() { } } +async function determineDefaultConverter() { + if (await commandExists('ffmpeg')) { + return 'ffmpeg' + } else if (await commandExists('avconv')) { + return 'avconv' + } else { + return null + } +} + async function main(args) { let sourcePlaylist = null let activePlaylist = null @@ -63,6 +73,7 @@ async function main(args) { let pickerLoopMode = 'loop-regenerate' let shuffleSeed let playerCommand = await determineDefaultPlayer() + let converterCommand = await determineDefaultConverter() // WILL play says whether the user has forced playback via an argument. // SHOULD play says whether the program has automatically decided to play @@ -70,6 +81,10 @@ async function main(args) { let shouldPlay = true let willPlay = null + // The same WILL/SHOULD rules apply here. + let shouldUseConverterOptions = true + let willUseConverterOptions = null + let disablePlaybackStatus = false const keybindings = [ @@ -116,7 +131,7 @@ async function main(args) { sourcePlaylist = processedPlaylist activePlaylist = clone(processedPlaylist) - processArgv(processedPlaylist.options, optionFunctions) + await processArgv(processedPlaylist.options, optionFunctions) } async function openKeybindings(arg, add = true) { @@ -378,6 +393,59 @@ async function main(args) { playerCommand = util.nextArg() }, + '-converter': async function(util) { + const command = util.nextArg() + + if (await commandExists(command)) { + converterCommand = command + } else { + console.warn(`Converter ${command} does not exist!`) + console.warn( + 'Because of this, track-specific converter options are being' + + ' disabled. (Use --enable-converter-options to force usage of' + + ' them.)' + ) + + shouldUseConverterOptions = false + } + }, + + '-baz': function() { + // --baz + // Debugger argument used to print a message as soon as this it is + // processed. Handy for making sure the arguments are being processed + // in the right order. + + console.log('Baz!') + }, + + '-foo': function(util) { + // --foo + // Similar to --baz, but logs the next argument rather than 'Baz!'. + + console.log(util.nextArg()) + }, + + '-enable-converter-options': function() { + // --enable-converter-options (alias: --use-converter-options) + // Forces usage of track-specific converter options. + + willUseConverterOptions = true + }, + + '-use-converter-options': util => util.alias('-enable-converter-options'), + + '-disable-converter-options': function() { + // --disable-converter-options (alias: --no-use-converter-options) + // Forces track-specific converter options to not be used. + + willUseConverterOptions = false + }, + + '-no-use-converter-options': util => { + return util.alias('-disable-converter-options') + }, + '-disable-playback-status': function() { // --disable-playback-status (alias: --hide-playback-status) // Hides the playback status line. @@ -401,8 +469,8 @@ async function main(args) { if (willPlay || (willPlay === null && shouldPlay)) { console.log(`Using sort: ${pickerSortMode} and loop: ${pickerLoopMode}.`) - console.log(`Using ${playerCommand} player.`) + console.log(`Using ${converterCommand} converter.`) const { promise: playPromise, @@ -415,7 +483,10 @@ async function main(args) { sort: pickerSortMode, seed: shuffleSeed }, - playerCommand, + playerCommand, converterCommand, + useConverterOptions: willUseConverterOptions || ( + willUseConverterOptions === null && shouldUseConverterOptions + ), disablePlaybackStatus }) diff --git a/todo.txt b/todo.txt index c97be0b..44caebb 100644 --- a/todo.txt +++ b/todo.txt @@ -355,3 +355,24 @@ TODO: Make the natural sort in crawl-local ignore capitalization case. TODO: Make the '@ ...' part of track-info show the path to the track, rather than the track name again (this is a bug!). (Done!) + +TODO: Only show the path to a given track's group in the track info. + +TODO: Figure out what to show in the '@ ...' part when a track is in the top + level (i.e. its group is only the top). Probably just '/'? + +TODO: Let track objects have an option for command line arguments to pass to + the converter program (ffmpeg or avconv). This would be useful for a + variety of cases: + * Have a track which has two minutes of blank audio you want to skip? + Make the converter skip past those first two minutes! + * Have a track which is much quieter than the rest of your library? + Make the converter amplify its volume! + It'd also be useful with the 'apply' option of groups (e.g., amplify + all the tracks in an album's group). + Since there are differences between avconv and ffmepg, it'd be + recommended to specify the converter program via the "options" part of + the playlist. When http-music finds that a playlist has asked for a + converter program that isn't installed, it simply won't run the + convertion options on the tracks in the playlist. + (Done!) -- cgit 1.3.0-6-gf8a5