Skip to content

Commit

Permalink
fix(audioconv): Improve ffmpeg bin check and error handling
Browse files Browse the repository at this point in the history
- Removed the promisified `exec` call for `ffmpeg -version` check and replaced it with a more reliable `spawnSync` approach.
- Refined the environment variable check for `FFMPEG_PATH` to ensure it is not null or undefined before proceeding.
- Enhanced `splitOptions` to correctly handle options without values, avoiding accidental concatenation with subsequent options.
- Streamlined error handling and logging for better clarity during the `ffmpeg` check process.
  • Loading branch information
mitsuki31 committed Aug 24, 2024
1 parent 3a66580 commit 92daed6
Showing 1 changed file with 18 additions and 26 deletions.
44 changes: 18 additions & 26 deletions lib/audioconv.js
Original file line number Diff line number Diff line change
Expand Up @@ -31,7 +31,6 @@ const fs = require('fs');
const path = require('path');
const childProcess = require('node:child_process');
const { EOL } = require('node:os');
const { promisify } = require('node:util');
const ffmpeg = require('fluent-ffmpeg');

const {
Expand All @@ -43,10 +42,6 @@ const {
isNullOrUndefined
} = require('./utils');

// Promisify the `exec` function
const exec = promisify(childProcess.exec);


/**
* Options for configuring the audio conversion.
*
Expand Down Expand Up @@ -129,7 +124,13 @@ function splitOptions(options) {
if (option.startsWith('-')) {
valueIndex = index + 1;
if (valueIndex < optionsList.length) {
resolvedOptions.push(`${option} ${optionsList[valueIndex]}`);
// Check if the next argument is not an option that starts with hyphen
if (!optionsList[valueIndex].startsWith('-')) {
resolvedOptions.push(`${option} ${optionsList[valueIndex]}`);
// Otherwise, only push the option without the next argument as value
} else {
resolvedOptions.push(option);
}
} else {
resolvedOptions.push(option);
}
Expand Down Expand Up @@ -216,7 +217,7 @@ function resolveOptions(options, useDefault=true) {
*/
async function checkFfmpeg(verbose=false) {
verbose && log.debug('Checking `ffmpeg` binary...');
if (process.env.FFMPEG_PATH || process.env.FFMPEG_PATH !== '') {
if (!isNullOrUndefined(process.env.FFMPEG_PATH) && process.env.FFMPEG_PATH !== '') {
if ((await fs.promises.stat(process.env.FFMPEG_PATH)).isDirectory()) {
const msg = '[EISDIR] Please set the FFMPEG_PATH environment variable '
+ 'to the path of the `ffmpeg` binary';
Expand All @@ -228,26 +229,17 @@ async function checkFfmpeg(verbose=false) {
return true;
}

// This try-catch block to handle error,
// in case the `ffmpeg` binary is not installed on system
try {
const { status } = await exec('ffmpeg -version');
if (status !== 0) {
verbose && log.debug('`ffmpeg` installed on system');
return true;
}

verbose && log.error('`ffmpeg` not installed on system');
return false;
/* eslint-disable-next-line no-unused-vars
---
Only need to ensure that the `spawn` call is correctly executed
to check FFmpeg; if error occurred, that means the `ffmpeg` command
are not recognized */
} catch (_err) {
verbose && log.error('`ffmpeg` not installed on system');
return false;
const { status } = childProcess.spawnSync('ffmpeg', ['-version'], {
shell: true, // For Windows, would cause error if this set to false
windowsHide: true
});
if (status === 0) {
verbose && log.debug('`ffmpeg` installed on system');
return true;
}

verbose && log.error('`ffmpeg` not installed on system');
return false;
}

function writeErrorLog(logFile, data, error) {
Expand Down

0 comments on commit 92daed6

Please sign in to comment.