Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add terminateTimeout option #1292

Open
wants to merge 2 commits into
base: master
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 2 additions & 1 deletion README.md
Original file line number Diff line number Diff line change
Expand Up @@ -85,7 +85,8 @@ var command = ffmpeg('/path/to/file.avi', { option: "value", ... });

The following options are available:
* `source`: input file name or readable stream (ignored if an input file is passed to the constructor)
* `timeout`: ffmpeg timeout in seconds (defaults to no timeout)
* `timeout`: ffmpeg process timeout in seconds (defaults to no timeout)
* `terminateTimeout`: ffmpeg terminate timeout in milliseconds (defaults to 20 milliseconds)
* `preset` or `presets`: directory to load module presets from (defaults to the `lib/presets` directory in fluent-ffmpeg tree)
* `niceness` or `priority`: ffmpeg niceness value, between -20 and 20; ignored on Windows platforms (defaults to 0)
* `logger`: logger object with `debug()`, `info()`, `warn()` and `error()` methods (defaults to no logging)
Expand Down
7 changes: 4 additions & 3 deletions lib/processor.js
Original file line number Diff line number Diff line change
Expand Up @@ -482,13 +482,14 @@ module.exports = function(proto) {
self.logger.debug('Output stream closed, scheduling kill for ffmpeg process');

// Don't kill process yet, to give a chance to ffmpeg to
// terminate successfully first This is necessary because
// terminate successfully first. This is necessary because
// under load, the process 'exit' event sometimes happens
// after the output stream 'close' event.
const terminateTimeout = (self.options && self.options.terminateTimeout > 0) ? self.options.terminateTimeout : 20
setTimeout(function() {
emitEnd(new Error('Output stream closed'));
emitEnd(new Error('Output stream closed, try increasing terminateTimeout option (' + terminateTimeout + ' ms)'));
ffmpegProc.kill();
}, 20);
}, terminateTimeout);
});

outputStream.target.on('error', function(err) {
Expand Down
48 changes: 45 additions & 3 deletions test/processor.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -232,7 +232,6 @@ describe('Processor', function() {
.on('end', function() {
console.log('end was called, expected a timeout');
assert.ok(false);
done();
})
.saveToFile(testFile);
});
Expand Down Expand Up @@ -279,6 +278,51 @@ describe('Processor', function() {
.saveToFile(testFile);
});

it('should kill the process on terminate timeout (terminateTimeout option)', function(done) {
this.timeout(60000);

var testFile = path.join(__dirname, 'assets', 'testTerminateTimeout.avi');
this.files.push(testFile);

const terminateTimeout = 1000 // set terminateTimeout to 1000 ms
var command = this.getCommand({
source: this.testfilebig,
logger: testhelper.logger,
terminateTimeout: terminateTimeout
});

var startCalled = false;
var outputStreamClosed = false;

// Mock output stream
var outputStream = new stream.PassThrough();
outputStream.close = function() {
outputStreamClosed = true;
this.emit('close');
};

command
.usingPreset('divx')
.output(outputStream)
.on('start', function() {
startCalled = true;
setTimeout(function() { outputStream.close(); }, 500); // close the output stream after 500ms (emit 'close' event on output stream before 'exit' event, to simulate a under load system)
})
.on('error', function(err) {
command.kill();
assert.ok(startCalled);
assert.ok(outputStreamClosed);
assert.equal(err.message, 'Output stream closed, try increasing terminateTimeout option (' + terminateTimeout + ' ms)');
// Wait for kill completation before to call done()
setTimeout(function() { done(); }, 500);
})
.on('end', function() {
console.log('end was called, expected an error');
assert.ok(false);
})
.saveToFile(testFile);
});

it('should not keep node process running on completion', function(done) {
var script = `
var ffmpeg = require('.');
Expand Down Expand Up @@ -320,7 +364,6 @@ describe('Processor', function() {
.on('end', function() {
console.log('end was called, expected an error');
assert.ok(false);
done();
})
.saveToFile(testFile);
});
Expand Down Expand Up @@ -357,7 +400,6 @@ describe('Processor', function() {
.on('end', function() {
console.log('end was called, expected a timeout');
assert.ok(false);
done();
})
.saveToFile(testFile);

Expand Down