Merge pull request #363 from kokorin/develop

Develop to Master

.github/workflows/codeql-analysis.yml

@@ -40,24 +40,24 @@ jobs:
 
     steps:
       - name: Checkout repository
-        uses: actions/checkout@v2
+        uses: actions/checkout@v4
 
-      - uses: actions/setup-java@v2
+      - uses: actions/setup-java@v3
         with:
           distribution: 'adopt'
-          java-version: 8
+          java-version: 17
 
       # Initializes the CodeQL tools for scanning.
       - name: Initialize CodeQL
-        uses: github/codeql-action/init@v1
+        uses: github/codeql-action/init@v2
         with:
           languages: ${{ matrix.language }}
           # If you wish to specify custom queries, you can do so here or in a config file.
           # By default, queries listed here will override any specified in a config file.
           # Prefix the list here with "+" to use these queries and those in the config file.
           # queries: ./path/to/local/query, your-org/your-repo/queries@main
 
-      - uses: actions/cache@v2
+      - uses: actions/cache@v3
         with:
           # be careful not to include ~/.m2/settings.xml which contains credentials
           path: |
@@ -70,4 +70,4 @@ jobs:
         run: bash mvnw clean install -B -DskipTests
 
       - name: Perform CodeQL Analysis
-        uses: github/codeql-action/analyze@v1
+        uses: github/codeql-action/analyze@v2

.github/workflows/release.yml

@@ -17,14 +17,14 @@ jobs:
     runs-on: ubuntu-latest
 
     steps:
-      - uses: actions/checkout@v2
+      - uses: actions/checkout@v4
 
-      - uses: actions/setup-java@v2
+      - uses: actions/setup-java@v3
         with:
           distribution: 'adopt'
           java-version: 11
 
-      - uses: actions/cache@v2
+      - uses: actions/cache@v3
         with:
           # be careful not to include ~/.m2/settings.xml which contains credentials
           path: |

.github/workflows/sonar-analysis.yml

@@ -26,26 +26,27 @@ jobs:
     runs-on: ubuntu-latest
 
     steps:
-      - uses: actions/checkout@v2
+      - uses: actions/checkout@v4
         with:
           fetch-depth: 0  # Shallow clones should be disabled for a better relevancy of analysis
 
-      - name: Set up JDK 11
-        uses: actions/setup-java@v1
+      - name: Set up JDK
+        uses: actions/setup-java@v3
         with:
-          java-version: 11
+          distribution: 'adopt'
+          java-version: 17
 
       - name: Install ffmpeg on Ubuntu
         run: sudo apt-get update && sudo apt-get install -y ffmpeg && ffmpeg -version
 
       - name: Cache SonarCloud packages
-        uses: actions/cache@v1
+        uses: actions/cache@v3
         with:
           path: ~/.sonar/cache
           key: ${{ runner.os }}-sonar
           restore-keys: ${{ runner.os }}-sonar
 
-      - uses: actions/cache@v2
+      - uses: actions/cache@v3
         with:
           # be careful not to include ~/.m2/settings.xml which contains credentials
           path: |

.github/workflows/tests.yml

@@ -30,16 +30,16 @@ jobs:
       fail-fast: false
 
     steps:
-      - uses: actions/checkout@v2
+      - uses: actions/checkout@v4
 
       - name: Set up JDK
-        uses: actions/setup-java@v2
+        uses: actions/setup-java@v3
         with:
           distribution: 'adopt'
           java-version: ${{ matrix.java-version }}
 
       - name: Cache Maven Packages
-        uses: actions/cache@v2
+        uses: actions/cache@v3
         with:
           # be careful not to include ~/.m2/settings.xml which contains credentials
           path: |
@@ -49,7 +49,7 @@ jobs:
           restore-keys: ${{ runner.os }}-m2
 
       - name: Cache Test Artifacts
-        uses: actions/cache@v2
+        uses: actions/cache@v3
         with:
           path: |
             .artifacts
@@ -82,18 +82,18 @@ jobs:
   test-release:
     runs-on: ubuntu-latest
     steps:
-      - uses: actions/checkout@v2
+      - uses: actions/checkout@v4
 
       - name: Set up JDK
-        uses: actions/setup-java@v2
+        uses: actions/setup-java@v3
         with:
           distribution: 'adopt'
           java-version: 11
 
       - name: Set up RANDOM GPG key
         run: gpg --quick-generate-key --batch  --passphrase '' test42
 
-      - uses: actions/cache@v2
+      - uses: actions/cache@v3
         with:
           # be careful not to include ~/.m2/settings.xml which contains credentials
           path: |

pom.xml

@@ -164,7 +164,7 @@
                     <plugin>
                         <groupId>org.apache.maven.plugins</groupId>
                         <artifactId>maven-javadoc-plugin</artifactId>
-                        <version>3.3.1</version>
+                        <version>3.5.0</version>
                         <configuration>
                             <source>8</source>
                         </configuration>
@@ -194,7 +194,7 @@
                     <plugin>
                         <groupId>org.sonatype.plugins</groupId>
                         <artifactId>nexus-staging-maven-plugin</artifactId>
-                        <version>1.6.8</version>
+                        <version>1.6.13</version>
                         <extensions>true</extensions>
                         <configuration>
                             <serverId>ossrh</serverId>
@@ -212,7 +212,7 @@
             <plugin>
                 <groupId>org.apache.maven.plugins</groupId>
                 <artifactId>maven-compiler-plugin</artifactId>
-                <version>3.8.1</version>
+                <version>3.11.0</version>
                 <configuration>
                     <source>8</source>
                     <target>8</target>
@@ -251,7 +251,7 @@
                     <dependency>
                         <groupId>com.puppycrawl.tools</groupId>
                         <artifactId>checkstyle</artifactId>
-                        <version>9.2.1</version>
+                        <version>10.12.3</version>
                     </dependency>
                 </dependencies>
                 <executions>

src/main/java/com/github/kokorin/jaffree/ffmpeg/FFmpeg.java

@@ -59,6 +59,7 @@ public class FFmpeg {
 
     private LogLevel logLevel = LogLevel.INFO;
     private String contextName = null;
+    private Integer executorTimeoutMillis = null;
 
     private final Path executable;
 
@@ -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>
@@ -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;
     }
 
     /**

src/main/java/com/github/kokorin/jaffree/process/ProcessHandler.java

@@ -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);
 
     /**
@@ -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>
@@ -172,15 +187,15 @@ public synchronized T execute() {
      */
     protected T interactWithProcess(final Process process) {
         AtomicReference<T> resultRef = new AtomicReference<>();
-        Integer status = null;
+        final int status;
         Executor executor = startExecution(process, resultRef);
 
         try {
             LOGGER.info("Waiting for process to finish");
             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) {
@@ -198,7 +213,7 @@ protected T interactWithProcess(final Process process) {
                     "Failed to execute, exception appeared in one of helper threads", exceptions);
         }
 
-        if (!Integer.valueOf(0).equals(status)) {
+        if (status != 0) {
             throw new JaffreeAbnormalExitException(
                 "Process execution has ended with non-zero status: " + status
                     + ". Check logs for detailed error message.",
@@ -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;

src/test/java/com/github/kokorin/jaffree/ffmpeg/FFmpegTest.java

@@ -13,6 +13,7 @@
 import org.hamcrest.core.AllOf;
 import org.hamcrest.core.StringContains;
 import org.junit.Assert;
+import org.junit.Assume;
 import org.junit.Ignore;
 import org.junit.Rule;
 import org.junit.Test;
@@ -379,16 +380,31 @@ public void testSizeLimit() throws Exception {
         Path tempDir = Files.createTempDirectory("jaffree");
         Path outputPath = tempDir.resolve(Artifacts.VIDEO_MP4.getFileName());
 
-        FFmpegResult result = FFmpeg.atPath(Config.FFMPEG_BIN)
-                .addInput(UrlInput.fromPath(Artifacts.VIDEO_MP4))
-                .addOutput(UrlOutput
-                        .toPath(outputPath)
-                        .copyAllCodecs()
-                        .setSizeLimit(1_000_000L)
-                )
-                .execute();
+        final AtomicBoolean muxingErrorDetected = new AtomicBoolean(false);
+        OutputListener outputListener = message -> {
+            if (message.endsWith("Error muxing a packet")) {
+                LOGGER.warn("Detected a muxing error, which indicates ffmpeg bug #10327");
+                muxingErrorDetected.set(true);
+            }
+        };
 
-        Assert.assertNotNull(result);
+        try {
+            FFmpegResult result = FFmpeg.atPath(Config.FFMPEG_BIN)
+                    .addInput(UrlInput.fromPath(Artifacts.VIDEO_MP4))
+                    .setOutputListener(outputListener)
+                    .addOutput(UrlOutput
+                            .toPath(outputPath)
+                            .copyAllCodecs()
+                            .setSizeLimit(1_000_000L)
+                    )
+                    .execute();
+            Assert.assertNotNull(result);
+        } catch (JaffreeAbnormalExitException ex) {
+            // Detect ffmpeg bug "Error muxing a packet when limit file size parameter is set"
+            // https://trac.ffmpeg.org/ticket/10327
+            Assume.assumeFalse("Hit ffmpeg bug #10327 - we will ignore this test", muxingErrorDetected.get());
+            Assert.fail("Abnormal exit for limit file size");
+        }
 
         long outputSize = Files.size(outputPath);
         assertTrue(outputSize > 900_000);