Skip to content

Commit

Permalink
Issue 355: ProcessHandler configurable executor timeout (#356)
Browse files Browse the repository at this point in the history
* Issue 355: Allow an application developer to override the default Executor Timeout value

* Issue 355: PR feedback: use int param instead of allowing nulls + 0 timeout means indefinite wait
  • Loading branch information
marklassau authored Sep 10, 2023
1 parent 96a47f1 commit 68d1341
Show file tree
Hide file tree
Showing 2 changed files with 51 additions and 9 deletions.
36 changes: 31 additions & 5 deletions src/main/java/com/github/kokorin/jaffree/ffmpeg/FFmpeg.java
Original file line number Diff line number Diff line change
Expand Up @@ -59,6 +59,7 @@ public class FFmpeg {

private LogLevel logLevel = LogLevel.INFO;
private String contextName = null;
private Integer executorTimeoutMillis = null;

private final Path executable;

Expand Down Expand Up @@ -387,6 +388,26 @@ public FFmpeg setContextName(final String contextName) {
return this;
}

/**
* Overrides the default {@link com.github.kokorin.jaffree.process.Executor} timeout.
* <p>
* Most normal use cases will easily complete within the default timeout. It is not recommended
* to set an explicit timeout value unless you have actually experienced unwanted timeouts.
* <p>
* A value of 0 will disable the timeout.
* That is, Jaffree will wait indefinitely for the Executor to complete.
*
* @param executorTimeoutMillis the custom executor timeout in milliseconds
* @return this
*/
public FFmpeg setExecutorTimeoutMillis(final int executorTimeoutMillis) {
if (executorTimeoutMillis < 0) {
throw new IllegalArgumentException("Executor timeout cannot be negative");
}
this.executorTimeoutMillis = executorTimeoutMillis;
return this;
}

/**
* Starts synchronous ffmpeg execution.
* <p>
Expand Down Expand Up @@ -480,11 +501,16 @@ protected ProcessHandler<FFmpegResult> createProcessHandler() {
helpers.add(progressHelper);
}

return new ProcessHandler<FFmpegResult>(executable, contextName)
.setStdErrReader(createStdErrReader(outputListener))
.setStdOutReader(createStdOutReader())
.setHelpers(helpers)
.setArguments(buildArguments());
ProcessHandler<FFmpegResult> processHandler =
new ProcessHandler<FFmpegResult>(executable, contextName)
.setStdErrReader(createStdErrReader(outputListener))
.setStdOutReader(createStdOutReader())
.setHelpers(helpers)
.setArguments(buildArguments());
if (executorTimeoutMillis != null) {
processHandler.setExecutorTimeoutMillis(executorTimeoutMillis);
}
return processHandler;
}

/**
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -49,8 +49,9 @@ public class ProcessHandler<T> {
private List<ProcessHelper> helpers = null;
private Stopper stopper = null;
private List<String> arguments = Collections.emptyList();
private int executorTimeoutMillis = DEFAULT_EXECUTOR_TIMEOUT_MILLIS;

private static final int EXECUTOR_TIMEOUT_MILLIS = 10_000;
private static final int DEFAULT_EXECUTOR_TIMEOUT_MILLIS = 10_000;
private static final Logger LOGGER = LoggerFactory.getLogger(ProcessHandler.class);

/**
Expand Down Expand Up @@ -120,6 +121,20 @@ public synchronized ProcessHandler<T> setArguments(final List<String> arguments)
return this;
}

/**
* Overrides the default Executor timeout.
* <p>
* A value of 0 is interpreted as "wait indefinitely".
*
* @param executorTimeoutMillis the new Executor timeout in milliseconds
*/
public void setExecutorTimeoutMillis(final int executorTimeoutMillis) {
if (executorTimeoutMillis < 0) {
throw new IllegalArgumentException("Executor timeout cannot be negative");
}
this.executorTimeoutMillis = executorTimeoutMillis;
}

/**
* Executes a program.
* <p>
Expand Down Expand Up @@ -180,7 +195,7 @@ protected T interactWithProcess(final Process process) {
status = process.waitFor();
LOGGER.info("Process has finished with status: {}", status);

waitForExecutorToStop(executor, EXECUTOR_TIMEOUT_MILLIS);
waitForExecutorToStop(executor, executorTimeoutMillis);
} catch (InterruptedException e) {
LOGGER.warn("Process has been interrupted");
if (stopper != null) {
Expand Down Expand Up @@ -308,9 +323,10 @@ private static void waitForExecutorToStop(final Executor executor, final long ti
throws InterruptedException {
LOGGER.debug("Waiting for Executor to stop");

long waitStarted = System.currentTimeMillis();
final long waitStarted = System.currentTimeMillis();
do {
if (System.currentTimeMillis() - waitStarted > timeoutMillis) {
// Zero timeout means "wait indefinitely"
if (timeoutMillis > 0 && System.currentTimeMillis() - waitStarted > timeoutMillis) {
LOGGER.warn("Executor hasn't stopped in {} millis, won't wait longer",
timeoutMillis);
break;
Expand Down

0 comments on commit 68d1341

Please sign in to comment.