Track-specific converter options

See todo.txt, changes in the man page, and this example:
https://gist.github.com/towerofnix/9b1853ee74d30357e1b59d22c59aba2f

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
@@ -80,10 +80,28 @@ 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.
 If this is the last option used, nothing plays (see \fB\-\-play\fR).
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!)