Merge pull request #2554 from mikiher/ffmpeg-latest

Modify BinaryManager to download version 6.1

server/managers/BinaryManager.js

@@ -1,3 +1,6 @@
+const child_process = require('child_process')
+const { promisify } = require('util')
+const exec = promisify(child_process.exec)
 const path = require('path')
 const which = require('../libs/which')
 const fs = require('../libs/fsExtra')
@@ -8,67 +11,143 @@ const fileUtils = require('../utils/fileUtils')
 class BinaryManager {
 
   defaultRequiredBinaries = [
-    { name: 'ffmpeg', envVariable: 'FFMPEG_PATH' },
+    { name: 'ffmpeg', envVariable: 'FFMPEG_PATH', validVersions: ['5.1', '6'] },
-    { name: 'ffprobe', envVariable: 'FFPROBE_PATH' }
+    { name: 'ffprobe', envVariable: 'FFPROBE_PATH', validVersions: ['5.1', '6'] }
   ]
 
   constructor(requiredBinaries = this.defaultRequiredBinaries) {
     this.requiredBinaries = requiredBinaries
     this.mainInstallPath = process.pkg ? path.dirname(process.execPath) : global.appRoot
     this.altInstallPath = global.ConfigPath
+    this.initialized = false
+    this.exec = exec
   }
 
   async init() {
     if (this.initialized) return
     const missingBinaries = await this.findRequiredBinaries()
     if (missingBinaries.length == 0) return
+    await this.removeOldBinaries(missingBinaries)
     await this.install(missingBinaries)
     const missingBinariesAfterInstall = await this.findRequiredBinaries()
-    if (missingBinariesAfterInstall.length != 0) {
+    if (missingBinariesAfterInstall.length) {
       Logger.error(`[BinaryManager] Failed to find or install required binaries: ${missingBinariesAfterInstall.join(', ')}`)
       process.exit(1)
     }
     this.initialized = true
   }
 
+  /**
+   * Remove old/invalid binaries in main or alt install path
+   * 
+   * @param {string[]} binaryNames 
+   */
+  async removeOldBinaries(binaryNames) {
+    for (const binaryName of binaryNames) {
+      const executable = this.getExecutableFileName(binaryName)
+      const mainInstallPath = path.join(this.mainInstallPath, executable)
+      if (await fs.pathExists(mainInstallPath)) {
+        Logger.debug(`[BinaryManager] Removing old binary: ${mainInstallPath}`)
+        await fs.remove(mainInstallPath)
+      }
+      const altInstallPath = path.join(this.altInstallPath, executable)
+      if (await fs.pathExists(altInstallPath)) {
+        Logger.debug(`[BinaryManager] Removing old binary: ${altInstallPath}`)
+        await fs.remove(altInstallPath)
+      }
+    }
+  }
+
+  /**
+   * Find required binaries and return array of binary names that are missing
+   * 
+   * @returns {Promise<string[]>}
+   */
   async findRequiredBinaries() {
     const missingBinaries = []
     for (const binary of this.requiredBinaries) {
-      const binaryPath = await this.findBinary(binary.name, binary.envVariable)
+      const binaryPath = await this.findBinary(binary.name, binary.envVariable, binary.validVersions)
       if (binaryPath) {
-        Logger.info(`[BinaryManager] Found ${binary.name} at ${binaryPath}`)
+        Logger.info(`[BinaryManager] Found valid binary ${binary.name} at ${binaryPath}`)
         if (process.env[binary.envVariable] !== binaryPath) {
           Logger.info(`[BinaryManager] Updating process.env.${binary.envVariable}`)
           process.env[binary.envVariable] = binaryPath
         }
       } else {
-        Logger.info(`[BinaryManager] ${binary.name} not found`)
+        Logger.info(`[BinaryManager] ${binary.name} not found or version too old`)
         missingBinaries.push(binary.name)
       }
     }
     return missingBinaries
   }
 
-  async findBinary(name, envVariable) {
+  /**
-    const executable = name + (process.platform == 'win32' ? '.exe' : '')
+   * Find absolute path for binary
+   * 
+   * @param {string} name 
+   * @param {string} envVariable 
+   * @param {string[]} [validVersions]
+   * @returns {Promise<string>} Path to binary
+   */
+  async findBinary(name, envVariable, validVersions = []) {
+    const executable = this.getExecutableFileName(name)
+    // 1. check path specified in environment variable
     const defaultPath = process.env[envVariable]
-    if (defaultPath && await fs.pathExists(defaultPath)) return defaultPath
+    if (await this.isBinaryGood(defaultPath, validVersions)) return defaultPath
+    // 2. find the first instance of the binary in the PATH environment variable
     const whichPath = which.sync(executable, { nothrow: true })
-    if (whichPath) return whichPath
+    if (await this.isBinaryGood(whichPath, validVersions)) return whichPath
+    // 3. check main install path (binary root dir)
     const mainInstallPath = path.join(this.mainInstallPath, executable)
-    if (await fs.pathExists(mainInstallPath)) return mainInstallPath
+    if (await this.isBinaryGood(mainInstallPath, validVersions)) return mainInstallPath
+    // 4. check alt install path (/config)
     const altInstallPath = path.join(this.altInstallPath, executable)
-    if (await fs.pathExists(altInstallPath)) return altInstallPath
+    if (await this.isBinaryGood(altInstallPath, validVersions)) return altInstallPath
     return null
   }
 
+  /**
+   * Check binary path exists and optionally check version is valid
+   * 
+   * @param {string} binaryPath 
+   * @param {string[]} [validVersions]
+   * @returns {Promise<boolean>}
+   */
+  async isBinaryGood(binaryPath, validVersions = []) {
+    if (!binaryPath || !await fs.pathExists(binaryPath)) return false
+    if (!validVersions.length) return true
+    try {
+      const { stdout } = await this.exec('"' + binaryPath + '"' + ' -version')
+      const version = stdout.match(/version\s([\d\.]+)/)?.[1]
+      if (!version) return false
+      return validVersions.some(validVersion => version.startsWith(validVersion))
+    } catch (err) {
+      Logger.error(`[BinaryManager] Failed to check version of ${binaryPath}`)
+      return false
+    }
+  }
+
+  /**
+   * 
+   * @param {string[]} binaries 
+   */
   async install(binaries) {
-    if (binaries.length == 0) return
+    if (!binaries.length) return
     Logger.info(`[BinaryManager] Installing binaries: ${binaries.join(', ')}`)
     let destination = await fileUtils.isWritable(this.mainInstallPath) ? this.mainInstallPath : this.altInstallPath
-    await ffbinaries.downloadBinaries(binaries, { destination })
+    await ffbinaries.downloadBinaries(binaries, { destination, version: '6.1', force: true })
     Logger.info(`[BinaryManager] Binaries installed to ${destination}`)
   }
+
+  /**
+   * Append .exe to binary name for Windows
+   * 
+   * @param {string} name 
+   * @returns {string}
+   */
+  getExecutableFileName(name) {
+    return name + (process.platform == 'win32' ? '.exe' : '')
+  }
 }
 
 module.exports = BinaryManager

server/utils/fileUtils.js

@@ -366,15 +366,16 @@ module.exports.encodeUriPath = (path) => {
  * This method is necessary because fs.access(directory, fs.constants.W_OK) does not work on Windows
  * 
  * @param {string} directory 
- * @returns {boolean}
+ * @returns {Promise<boolean>}
  */
 module.exports.isWritable = async (directory) => {
   try {
-    const accessTestFile = path.join(directory, 'accessTest')
+    const accessTestFile = Path.join(directory, 'accessTest')
     await fs.writeFile(accessTestFile, '')
     await fs.remove(accessTestFile)
     return true
   } catch (err) {
+    Logger.info(`[fileUtils] Directory is not writable "${directory}"`, err)
     return false
   }
 }

test/server/managers/BinaryManager.test.js

@@ -15,6 +15,7 @@ describe('BinaryManager', () => {
   describe('init', () => {
     let findStub
     let installStub
+    let removeOldBinariesStub
     let errorStub
     let exitStub
 
@@ -22,6 +23,7 @@ describe('BinaryManager', () => {
       binaryManager = new BinaryManager()
       findStub = sinon.stub(binaryManager, 'findRequiredBinaries')
       installStub = sinon.stub(binaryManager, 'install')
+      removeOldBinariesStub = sinon.stub(binaryManager, 'removeOldBinaries')
       errorStub = sinon.stub(console, 'error')
       exitStub = sinon.stub(process, 'exit')
     })
@@ -29,6 +31,7 @@ describe('BinaryManager', () => {
     afterEach(() => {
       findStub.restore()
       installStub.restore()
+      removeOldBinariesStub.restore()
       errorStub.restore()
       exitStub.restore()
     })
@@ -39,6 +42,7 @@ describe('BinaryManager', () => {
       await binaryManager.init()
 
       expect(installStub.called).to.be.false
+      expect(removeOldBinariesStub.called).to.be.false
       expect(findStub.calledOnce).to.be.true
       expect(errorStub.called).to.be.false
       expect(exitStub.called).to.be.false
@@ -54,6 +58,7 @@ describe('BinaryManager', () => {
 
       expect(findStub.calledTwice).to.be.true
       expect(installStub.calledOnce).to.be.true
+      expect(removeOldBinariesStub.calledOnce).to.be.true
       expect(errorStub.called).to.be.false
       expect(exitStub.called).to.be.false
     })
@@ -68,6 +73,7 @@ describe('BinaryManager', () => {
 
       expect(findStub.calledTwice).to.be.true
       expect(installStub.calledOnce).to.be.true
+      expect(removeOldBinariesStub.calledOnce).to.be.true
       expect(errorStub.calledOnce).to.be.true
       expect(exitStub.calledOnce).to.be.true
       expect(exitStub.calledWith(1)).to.be.true
@@ -171,7 +177,7 @@ describe('BinaryManager', () => {
 
 describe('findBinary', () => {
   let binaryManager
-  let fsPathExistsStub
+  let isBinaryGoodStub
   let whichSyncStub
   let mainInstallPath
   let altInstallPath
@@ -185,7 +191,7 @@ describe('findBinary', () => {
 
   beforeEach(() => {
     binaryManager = new BinaryManager()
-    fsPathExistsStub = sinon.stub(fs, 'pathExists')
+    isBinaryGoodStub = sinon.stub(binaryManager, 'isBinaryGood')
     whichSyncStub = sinon.stub(which, 'sync')
     binaryManager.mainInstallPath = '/path/to/main/install'
     mainInstallPath = path.join(binaryManager.mainInstallPath, executable)
@@ -194,71 +200,182 @@ describe('findBinary', () => {
   })
 
   afterEach(() => {
-    fsPathExistsStub.restore()
+    isBinaryGoodStub.restore()
     whichSyncStub.restore()
   })
   
-  it('should return defaultPath if it exists', async () => {
+  it('should return the defaultPath if it exists and is a good binary', async () => {
     process.env[envVariable] = defaultPath
-    fsPathExistsStub.withArgs(defaultPath).resolves(true)
+    isBinaryGoodStub.withArgs(defaultPath).resolves(true)
   
     const result = await binaryManager.findBinary(name, envVariable)
   
     expect(result).to.equal(defaultPath)    
-    expect(fsPathExistsStub.calledOnceWith(defaultPath)).to.be.true
+    expect(isBinaryGoodStub.calledOnce).to.be.true
-    expect(whichSyncStub.notCalled).to.be.true
+    expect(isBinaryGoodStub.calledWith(defaultPath)).to.be.true
   })
   
-  it('should return whichPath if it exists', async () => {
+  it('should return the whichPath if it exists and is a good binary', async () => {
     delete process.env[envVariable]
+    isBinaryGoodStub.withArgs(undefined).resolves(false)
+    isBinaryGoodStub.withArgs(whichPath).resolves(true)
     whichSyncStub.returns(whichPath)
   
     const result = await binaryManager.findBinary(name, envVariable)
   
     expect(result).to.equal(whichPath)
-    expect(fsPathExistsStub.notCalled).to.be.true
+    expect(isBinaryGoodStub.calledTwice).to.be.true
-    expect(whichSyncStub.calledOnce).to.be.true
+    expect(isBinaryGoodStub.calledWith(undefined)).to.be.true
+    expect(isBinaryGoodStub.calledWith(whichPath)).to.be.true
   })
   
-  it('should return mainInstallPath if it exists', async () => {
+  it('should return the mainInstallPath if it exists and is a good binary', async () => {
     delete process.env[envVariable]
+    isBinaryGoodStub.withArgs(undefined).resolves(false)
+    isBinaryGoodStub.withArgs(null).resolves(false)
+    isBinaryGoodStub.withArgs(mainInstallPath).resolves(true)
     whichSyncStub.returns(null)
-    fsPathExistsStub.withArgs(mainInstallPath).resolves(true)
   
     const result = await binaryManager.findBinary(name, envVariable)
   
     expect(result).to.equal(mainInstallPath)
-    expect(whichSyncStub.calledOnce).to.be.true
+    expect(isBinaryGoodStub.callCount).to.be.equal(3)
-    expect(fsPathExistsStub.calledOnceWith(mainInstallPath)).to.be.true
+    expect(isBinaryGoodStub.calledWith(undefined)).to.be.true
+    expect(isBinaryGoodStub.calledWith(null)).to.be.true
+    expect(isBinaryGoodStub.calledWith(mainInstallPath)).to.be.true
   })
   
-  it('should return altInstallPath if it exists', async () => {
+  it('should return the altInstallPath if it exists and is a good binary', async () => {
     delete process.env[envVariable]
+    isBinaryGoodStub.withArgs(undefined).resolves(false)
+    isBinaryGoodStub.withArgs(null).resolves(false)
+    isBinaryGoodStub.withArgs(mainInstallPath).resolves(false)
+    isBinaryGoodStub.withArgs(altInstallPath).resolves(true)
     whichSyncStub.returns(null)
-    fsPathExistsStub.withArgs(mainInstallPath).resolves(false)
-    fsPathExistsStub.withArgs(altInstallPath).resolves(true)
   
     const result = await binaryManager.findBinary(name, envVariable)
   
     expect(result).to.equal(altInstallPath)
-    expect(whichSyncStub.calledOnce).to.be.true
+    expect(isBinaryGoodStub.callCount).to.be.equal(4)
-    expect(fsPathExistsStub.calledTwice).to.be.true
+    expect(isBinaryGoodStub.calledWith(undefined)).to.be.true
-    expect(fsPathExistsStub.calledWith(mainInstallPath)).to.be.true
+    expect(isBinaryGoodStub.calledWith(null)).to.be.true
-    expect(fsPathExistsStub.calledWith(altInstallPath)).to.be.true
+    expect(isBinaryGoodStub.calledWith(mainInstallPath)).to.be.true
+    expect(isBinaryGoodStub.calledWith(altInstallPath)).to.be.true
   })
   
-  it('should return null if binary is not found', async () => {
+  it('should return null if no good binary is found', async () => {
     delete process.env[envVariable]
+    isBinaryGoodStub.withArgs(undefined).resolves(false)
+    isBinaryGoodStub.withArgs(null).resolves(false)
+    isBinaryGoodStub.withArgs(mainInstallPath).resolves(false)
+    isBinaryGoodStub.withArgs(altInstallPath).resolves(false)
     whichSyncStub.returns(null)
-    fsPathExistsStub.withArgs(mainInstallPath).resolves(false)
-    fsPathExistsStub.withArgs(altInstallPath).resolves(false)
   
     const result = await binaryManager.findBinary(name, envVariable)
   
     expect(result).to.be.null
-    expect(whichSyncStub.calledOnce).to.be.true
+    expect(isBinaryGoodStub.callCount).to.be.equal(4)
-    expect(fsPathExistsStub.calledTwice).to.be.true
+    expect(isBinaryGoodStub.calledWith(undefined)).to.be.true
-    expect(fsPathExistsStub.calledWith(mainInstallPath)).to.be.true
+    expect(isBinaryGoodStub.calledWith(null)).to.be.true
-    expect(fsPathExistsStub.calledWith(altInstallPath)).to.be.true
+    expect(isBinaryGoodStub.calledWith(mainInstallPath)).to.be.true
+    expect(isBinaryGoodStub.calledWith(altInstallPath)).to.be.true
+  })  
+})
+
+describe('isBinaryGood', () => {
+  let binaryManager
+  let fsPathExistsStub
+  let execStub
+  let loggerInfoStub
+  let loggerErrorStub
+
+  const binaryPath = '/path/to/binary'
+  const execCommand = '"' + binaryPath + '"' + ' -version'
+
+  beforeEach(() => {
+    binaryManager = new BinaryManager()
+    fsPathExistsStub = sinon.stub(fs, 'pathExists')
+    execStub = sinon.stub(binaryManager, 'exec')
+  })
+
+  afterEach(() => {
+    fsPathExistsStub.restore()
+    execStub.restore()
+  })
+
+  it('should return false if binaryPath is falsy', async () => {
+    fsPathExistsStub.resolves(true)
+
+    const result = await binaryManager.isBinaryGood(null)
+
+    expect(result).to.be.false
+    expect(fsPathExistsStub.called).to.be.false
+    expect(execStub.called).to.be.false
+  })
+
+  it('should return false if binaryPath does not exist', async () => {
+    fsPathExistsStub.resolves(false)
+
+    const result = await binaryManager.isBinaryGood(binaryPath)
+
+    expect(result).to.be.false
+    expect(fsPathExistsStub.calledOnce).to.be.true
+    expect(fsPathExistsStub.calledWith(binaryPath)).to.be.true
+    expect(execStub.called).to.be.false
+  })
+
+  it('should return false if failed to check version of binary', async () => {
+    fsPathExistsStub.resolves(true)
+    execStub.rejects(new Error('Failed to execute command'))
+
+    const result = await binaryManager.isBinaryGood(binaryPath)
+
+    expect(result).to.be.false
+    expect(fsPathExistsStub.calledOnce).to.be.true
+    expect(fsPathExistsStub.calledWith(binaryPath)).to.be.true
+    expect(execStub.calledOnce).to.be.true
+    expect(execStub.calledWith(execCommand)).to.be.true
+  })
+
+  it('should return false if version is not found', async () => {
+    const stdout = 'Some output without version'
+    fsPathExistsStub.resolves(true)
+    execStub.resolves({ stdout })
+
+    const result = await binaryManager.isBinaryGood(binaryPath)
+
+    expect(result).to.be.false
+    expect(fsPathExistsStub.calledOnce).to.be.true
+    expect(fsPathExistsStub.calledWith(binaryPath)).to.be.true
+    expect(execStub.calledOnce).to.be.true
+    expect(execStub.calledWith(execCommand)).to.be.true
+  })
+
+  it('should return false if version is found but does not match a good version', async () => {
+    const stdout = 'version 1.2.3'
+    fsPathExistsStub.resolves(true)
+    execStub.resolves({ stdout })
+
+    const result = await binaryManager.isBinaryGood(binaryPath)
+
+    expect(result).to.be.false
+    expect(fsPathExistsStub.calledOnce).to.be.true
+    expect(fsPathExistsStub.calledWith(binaryPath)).to.be.true
+    expect(execStub.calledOnce).to.be.true
+    expect(execStub.calledWith(execCommand)).to.be.true
+  })
+
+  it('should return true if version is found and matches a good version', async () => {
+    const stdout = 'version 6.1.2'
+    fsPathExistsStub.resolves(true)
+    execStub.resolves({ stdout })
+
+    const result = await binaryManager.isBinaryGood(binaryPath)
+
+    expect(result).to.be.true
+    expect(fsPathExistsStub.calledOnce).to.be.true
+    expect(fsPathExistsStub.calledWith(binaryPath)).to.be.true
+    expect(execStub.calledOnce).to.be.true
+    expect(execStub.calledWith(execCommand)).to.be.true
   })
 })