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

Issue 355: ProcessHandler configurable executor timeout #356

Merged
merged 2 commits into from
Sep 10, 2023
Merged
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
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;
kokorin marked this conversation as resolved.
Show resolved Hide resolved
}

/**
* 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);
kokorin marked this conversation as resolved.
Show resolved Hide resolved
} 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
Loading