Skip to content

Commit

Permalink
Improve and fix issue in audioconv module (#40)
Browse files Browse the repository at this point in the history 
- 91365f8 - docs(audioconv): Enhance JSDoc docs and update typedefs
- 84a4735 - fix(audioconv): Ensure error logging works on `quiet` enabled
- 92daed6 - fix(audioconv): Improve `ffmpeg` bin check and error handling

Signed-off-by: Ryuu Mitsuki <dhefam31@gmail.com>
  • Loading branch information
@mitsuki31
mitsuki31 committed Aug 24, 2024
2 parents 3a66580 + 91365f8 commit c2bc654
Show file tree
Hide file tree
Showing 2 changed files with 143 additions and 63 deletions.
186 changes: 126 additions & 60 deletions lib/audioconv.js
View file
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,16 +42,12 @@ const {
isNullOrUndefined
} = require('./utils');

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


/**
* Options for configuring the audio conversion.
*
* @typedef {Object} ConvertAudioOptions
* @property {string[]} [inputOptions] - The input options for the conversion.
* @property {string[]} [outputOptions] - The output options for the conversion.
* @property {string[]} [inputOptions=[]] - The input options for the conversion.
* @property {string[]} [outputOptions=[]] - The output options for the conversion.
* @property {string} [format='mp3'] - The desired output format (e.g., `'mp3'`, `'aac'`).
* @property {string | number} [bitrate=128] - The audio bitrate (e.g., `'128k'`),
* it may be a number or a string with an optional `k` suffix.
Expand Down Expand Up @@ -82,24 +77,39 @@ const exec = promisify(childProcess.exec);
* @property {boolean} deleteOld - Whether to delete the original file after conversion.
* @property {boolean} quiet - Whether to suppress the conversion progress and error message or not.
*
* @package
* @since 1.0.0
* @see {@link module:audioconv~defaultOptions defaultOptions}
*/

/**
* An object representing the information data when FFmpeg emits the `'progress'` event.
*
* @typedef {Object} FFmpegInfo
* @property {number} frames - Total processed frame count.
* @property {number} currentFps - Framerate at which FFmpeg is currently processing.
* @property {number} currentKbps - Throughput at which FFmpeg is currently processing.
* @property {number} targetSize - Current size of the target file in kilobytes.
* @property {number} timemark - The timestamp of the current frame in seconds.
* @property {number} percent - An estimation of the progress percentage, may be (very) inaccurate.
*
* @package
* @since 1.1.0
* @see ['progress' event]{@linkplain https://github.com/fluent-ffmpeg/node-fluent-ffmpeg#progress-transcoding-progress-information}
*/

/**
* Default options of audio converter options.
*
* Any option that are not specified on `options` argument in `convertAudio` function
* will fallback to this options. This default options will convert the audio
* to the MP3 format with bitrate of 128 kbps, frequency of 44100 Hz (Hertz),
* stereo channel and use the default MP3 codec.
* This default options will convert the audio to the MP3 format with bitrate of 128 kbps,
* frequency of 44100 Hz (Hertz), stereo channel and use the default MP3 codec.
*
* If you want to delete the old audio file after conversion, set the
* `deleteOld` option to `true`.
*
* @constant
* @type {ResolvedConvertAudioOptions}
* @public
* @readonly
* @type {Readonly<module:audioconv~ResolvedConvertAudioOptions>}
* @since 0.2.0
*/
const defaultOptions = Object.freeze({
Expand All @@ -116,6 +126,23 @@ const defaultOptions = Object.freeze({

// region Utilities

/**
* Splits and resolves FFmpeg options from a string or array format into an array of individual options.
*
* This function handles both single string input, where options are space-separated, and array input.
* It correctly pairs options with their respective values and avoids accidental concatenation with subsequent options.
*
* @example
* const optionsStr = '-f -vcodec libx264 -preset slow';
* const result1 = splitOptions(optionsStr);
* // Output: ['-f', '-vcodec libx264', '-preset slow']
*
* @param {string | string[]} options - The options to split, either as a string or an array.
* @returns {string[]} The resolved options as an array of individual options.
*
* @package
* @since 1.0.0
*/
function splitOptions(options) {
if (typeof options === 'string') {
const optionsList = options.trim().split(' ');
Expand All @@ -129,7 +156,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 All @@ -155,9 +188,9 @@ function splitOptions(options) {
/**
* Resolves the given {@link ConvertAudioOptions} options.
*
* @private
* @package
* @param {ConvertAudioOptions} options - The unresolved audio converter options.
* @return {ResolvedConvertAudioOptions} The resolved options.
* @return {module:audioconv~ResolvedConvertAudioOptions} The resolved options.
* @since 1.0.0
*/
function resolveOptions(options, useDefault=true) {
Expand Down Expand Up @@ -216,7 +249,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,51 +261,83 @@ 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) {
// Throw the error if the log file name is invalid
if (isNullOrUndefined(logFile) || typeof logFile !== 'string') {
throw error;
}
/**
* Writes error details and associated video information to a log file.
*
* The error message is written to the log file in the following format:
*
* ```txt
* [ERROR]<ACONV> <error message>
* Input Audio: <input audio name>
* Output Audio: <output audio name>
* File Size: <input audio size> MiB
* ---------------------------------------------
* ```
*
* Generated log file will be saved in {@link module:utils~LOGDIR `LOGDIR`} directory
* with file name typically prefixed with `'audioConvError'`.
*
* @param {string} logFile - The name of the log file where the error details should be written.
* @param {Object} data - An object containing information about the audio associated with the error.
* @param {Error} [error] - The error object, optional. If not provided,
* an error message will be `'Unknown error'`.
* @returns {Promise<void>}
*
* @async
* @package
* @since 1.0.0
*/
async function writeErrorLog(logFile, data, error) {
// Return immediately if given log file is not a string type
if (isNullOrUndefined(logFile) || typeof logFile !== 'string') return;

logFile = path.join(LOGDIR, path.basename(logFile));
createDirIfNotExistSync(LOGDIR);

const logStream = fs.createWriteStream(logFile);
return new Promise((resolve, reject) => {
const logStream = fs.createWriteStream(logFile, { flags: 'a+', flush: true });

logStream.write(`[ERROR]<ACONV> ${error?.message || 'Unknown error'}${EOL}`);
logStream.write(` Input Audio: ${data?.inputAudio || 'Unknown'}${EOL}`);
logStream.write(` Output Audio: ${data?.outputAudio || 'Unknown'}${EOL}`);
logStream.write(` File Size: ${data?.inputSize / (1024 * 1024) || '0.0'} MiB${EOL}`);
logStream.write(`---------------------------------------------${EOL}`);
logStream.end(EOL);

logStream.write(`[ERROR]<ACONV> ${error?.message || 'Unknown error'}${EOL}`);
logStream.write(` Input Audio: ${data?.inputAudio || 'Unknown'}${EOL}`);
logStream.write(` Output Audio: ${data?.outputAudio || 'Unknown'}${EOL}`);
logStream.write(` File Size: ${data?.inputSize / (1024 * 1024) || '0.0'} MiB${EOL}`);
logStream.write(`---------------------------------------------${EOL}`);
logStream.end(EOL);
logStream.on('finish', () => resolve());
logStream.on('error', (err) => {
if (!logStream.destroyed) logStream.destroy();
reject(err);
});
});
}


// region Audio Conversion


/**
* Creates a string representing the progress bar for audio conversion progress.
*
* @param {FFmpegInfo} info - The progress data from FFmpeg.
* @param {string[]} extnames - A list of extension names of both input and output files.
* @returns {string} A formatted string representing the progress bar with percentage.
*
* @package
* @since 1.0.0
*/
function createConversionProgress(info, extnames) {
const percentage = Math.max(0, Math.round(info.percent || 0));
const currentKbps = Math.max(0, info.currentKbps || 0);
Expand Down Expand Up @@ -411,17 +476,18 @@ async function convertAudio(inFile, options = defaultOptions) {
// Handlers
ffmpegChain
.on('error', (err) => {
if (!quiet) {
process.stdout.write('\n');
writeErrorLog(createLogFile('audioConvError'), {
inputFile: inFile,
outputFile: outFile,
inputSize: fs.statSync(inFile).size
}, err);
log.error('Failed to convert the audio file');
console.error('Caused by:', err.message);
}
reject(err);
quiet || process.stdout.write('\n');
writeErrorLog(createLogFile('audioConvError'), {
inputFile: inFile,
outputFile: outFile,
inputSize: fs.statSync(inFile).size
}, err).then(() => {
if (!quiet) {
log.error('Failed to convert the audio file');
console.error('Caused by:', err.message);
}
reject(err);
}).catch((errLog) => reject(new Error(errLog.message, { cause: err })));
})
.on('progress', (info) => {
// Write the progress information to the console
Expand Down
20 changes: 17 additions & 3 deletions lib/utils.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -69,11 +69,25 @@ const FrozenProperty = {
enumerable: true
};

/** The root directory of the project. */
/**
* The root directory of the project.
* @since 1.0.0
*/
const ROOTDIR = path.join(__dirname, '..');
/** The output directory for the downloaded audio files. */
/**
* The output directory for the downloaded audio files.
* @since 1.0.0
*/
const OUTDIR = path.join(ROOTDIR, 'download');
/** The log directory for the download error logs. */
/**
* The log directory for error logs.
*
* Basically, the log directory is set to:
* - **POSIX**: `$HOME/.ytmp3-js/logs`
* - **Windows**: `%USERPROFILE%\.ytmp3-js\logs`
*
* @since 1.0.0
*/
const LOGDIR = path.join(os.homedir(), '.ytmp3-js', 'logs');


Expand Down

0 comments on commit c2bc654

Please sign in to comment.