Merge remote-tracking branch 'upstream/master' into issue/138-build-f…

…fprobe-command
bramp · Mar 11, 2024 · fd67f31 · fd67f31
2 parents 94ce47b + be9d457
commit fd67f31
Show file tree

Hide file tree

Showing 12 changed files with 225 additions and 64 deletions.
diff --git a/.github/workflows/maven.yml b/.github/workflows/maven.yml
@@ -15,10 +15,7 @@ jobs:
     strategy:
       matrix:
         # Long term supported versions
-        java-version: [11, 17, 21]
-
-        # TODO Add support for 8, but it fails with
-        # java.lang.UnsupportedClassVersionError: com/spotify/fmt/FMT has been compiled by a more recent version of the Java Runtime (class file version 55.0), this version of the Java Runtime only recognizes class file versions up to 52.0
+        java-version: [8, 11, 17, 21]
 
         # TODO Should we test locales? The old travis setup did, see:
         # https://github.com/bramp/ffmpeg-cli-wrapper/pull/55
@@ -49,4 +46,5 @@ jobs:
 
     # Optional: Uploads the full dependency graph to GitHub to improve the quality of Dependabot alerts this repository can receive
     - name: Update dependency graph
-      uses: advanced-security/maven-dependency-submission-action@571e99aab1055c2e71a1e2309b9691de18d6b7d6
+      uses: advanced-security/maven-dependency-submission-action@v4
+      continue-on-error: true
diff --git a/pom.xml b/pom.xml
@@ -39,7 +39,9 @@
     </developers>
 
     <properties>
-        <base.java.version>1.8</base.java.version>
+        <base.java.version>8</base.java.version>
+        <maven.compiler.source>${base.java.version}</maven.compiler.source>
+        <maven.compiler.target>${base.java.version}</maven.compiler.target>
 
         <powermock.version>1.5.2</powermock.version>
 
@@ -125,7 +127,7 @@
             <dependency>
                 <groupId>org.glassfish.grizzly</groupId>
                 <artifactId>grizzly-http-server</artifactId>
-                <version>4.0.0</version>
+                <version>4.0.2</version>
             </dependency>
         </dependencies>
     </dependencyManagement>
@@ -243,11 +245,6 @@
                     <artifactId>maven-surefire-plugin</artifactId>
                     <version>3.2.1</version>
                 </plugin>
-                <plugin>
-                    <groupId>org.codehaus.mojo</groupId>
-                    <artifactId>animal-sniffer-maven-plugin</artifactId>
-                    <version>1.23</version>
-                </plugin>
                 <plugin>
                     <groupId>org.codehaus.mojo</groupId>
                     <artifactId>cobertura-maven-plugin</artifactId>
@@ -485,11 +482,6 @@
                         <artifactId>extra-enforcer-rules</artifactId>
                         <version>1.5.1</version>
                     </dependency>
-                    <dependency>
-                        <groupId>org.codehaus.mojo</groupId>
-                        <artifactId>animal-sniffer-enforcer-rule</artifactId>
-                        <version>1.21</version>
-                    </dependency>
                 </dependencies>
                 <executions>
                     <execution>
@@ -520,27 +512,6 @@
                 </executions>
             </plugin>
 
-            <plugin>
-                <groupId>org.codehaus.mojo</groupId>
-                <artifactId>animal-sniffer-maven-plugin</artifactId>
-                <executions>
-                    <execution>
-                        <id>check-java-api</id>
-                        <phase>test</phase>
-                        <goals>
-                            <goal>check</goal>
-                        </goals>
-                        <configuration>
-                            <signature>
-                                <groupId>org.codehaus.mojo.signature</groupId>
-                                <artifactId>java17</artifactId>
-                                <version>1.0</version>
-                            </signature>
-                        </configuration>
-                    </execution>
-                </executions>
-            </plugin>
-
             <!-- Coverage reporting -->
             <plugin>
                 <groupId>org.codehaus.mojo</groupId>
@@ -560,17 +531,6 @@
                     <sourceEncoding>UTF-8</sourceEncoding>
                 </configuration>
             </plugin>
-            <plugin>
-                <groupId>com.spotify.fmt</groupId>
-                <artifactId>fmt-maven-plugin</artifactId>
-                <executions>
-                    <execution>
-                        <goals>
-                            <goal>format</goal>
-                        </goals>
-                    </execution>
-                </executions>
-            </plugin>
         </plugins>
 
         <defaultGoal>install</defaultGoal>
@@ -740,4 +700,67 @@
 			</plugin>
 		</plugins>
 	</reporting>
+
+    <profiles>
+        <profile>
+            <!-- Some of the plugins and test deps have stopped working for Java
+                 8, so we specify old versions here.  This is so we can continue
+                 to run tests on Java 8, but allow the default version to use
+                 the latest versions. (As long as API is compatible with Java 8) -->
+            <id>Java 8</id>
+            <activation>
+                <jdk>1.8</jdk>
+            </activation>
+            <dependencyManagement>
+                <dependencies>
+                    <!-- Test Scope -->
+                    <dependency>
+                        <groupId>ch.qos.logback</groupId>
+                        <artifactId>logback-classic</artifactId>
+                        <!-- logback 1.4 requires JDK 11+ -->
+                        <version>1.3.14</version>
+                    </dependency>
+                    <dependency>
+                        <groupId>org.mockito</groupId>
+                        <artifactId>mockito-core</artifactId>
+                        <!-- mockito 5 requires JDK 11+ -->
+                        <version>4.11.0</version>
+                    </dependency>
+                    <dependency>
+                        <groupId>org.glassfish.grizzly</groupId>
+                        <artifactId>grizzly-http-server</artifactId>
+                        <!-- grizzly 4.0.0 requires JDK 11+ -->
+                        <version>3.0.1</version>
+                    </dependency>
+                </dependencies>
+            </dependencyManagement>
+
+        </profile>
+        <profile>
+            <id>Java 9+</id>
+            <activation>
+                <jdk>[9,)</jdk>
+            </activation>
+            <properties>
+                <!-- This flag was added from 9+ -->
+                <maven.compiler.release>${base.java.version}</maven.compiler.release>
+            </properties>
+            <build>
+                <pluginManagement>
+                    <plugins>
+                        <plugin>
+                            <groupId>com.spotify.fmt</groupId>
+                            <artifactId>fmt-maven-plugin</artifactId>
+                            <!-- Starting from version 1.8, Google Java
+                            Formatter requires Java 11 to run. Incidently, all
+                            versions of this plugin starting from 2.10
+                            inclusively also require this Java version to
+                            properly function.
+                            -->
+                        </plugin>
+                    </plugins>
+                </pluginManagement>
+            </build>
+        </profile>
+    </profiles>
 </project>
diff --git a/src/main/java/net/bramp/ffmpeg/FFcommon.java b/src/main/java/net/bramp/ffmpeg/FFcommon.java
@@ -8,6 +8,7 @@
 import com.google.common.io.CharStreams;
 import java.io.BufferedReader;
 import java.io.IOException;
+import java.io.InputStream;
 import java.io.InputStreamReader;
 import java.nio.charset.StandardCharsets;
 import java.util.List;
@@ -28,6 +29,12 @@ abstract class FFcommon {
   /** Version string */
   String version = null;
 
+  /** Process input stream */
+  Appendable processOutputStream = System.out;
+
+  /** Process error stream */
+  Appendable processErrorStream = System.err;
+
   public FFcommon(@Nonnull String path) {
     this(path, new RunProcessFunction());
   }
@@ -38,8 +45,26 @@ protected FFcommon(@Nonnull String path, @Nonnull ProcessFunction runFunction) {
     this.path = path;
   }
 
+  public void setProcessOutputStream(@Nonnull Appendable processOutputStream) {
+    Preconditions.checkNotNull(processOutputStream);
+    this.processOutputStream = processOutputStream;
+  }
+
+  public void setProcessErrorStream(@Nonnull Appendable processErrorStream) {
+    Preconditions.checkNotNull(processErrorStream);
+    this.processErrorStream = processErrorStream;
+  }
+
+  private BufferedReader _wrapInReader(final InputStream inputStream) {
+    return new BufferedReader(new InputStreamReader(inputStream, StandardCharsets.UTF_8));
+  }
+
   protected BufferedReader wrapInReader(Process p) {
-    return new BufferedReader(new InputStreamReader(p.getInputStream(), StandardCharsets.UTF_8));
+    return _wrapInReader(p.getInputStream());
+  }
+
+  protected BufferedReader wrapErrorInReader(Process p) {
+    return _wrapInReader(p.getErrorStream());
   }
 
   protected void throwOnError(Process p) throws IOException {
@@ -107,8 +132,8 @@ public void run(List<String> args) throws IOException {
       // TODO Move the copy onto a thread, so that FFmpegProgressListener can be on this thread.
 
       // Now block reading ffmpeg's stdout. We are effectively throwing away the output.
-      CharStreams.copy(wrapInReader(p), System.out); // TODO Should I be outputting to stdout?
-
+      CharStreams.copy(wrapInReader(p), processOutputStream);
+      CharStreams.copy(wrapErrorInReader(p), processErrorStream);
       throwOnError(p);
 
     } finally {

diff --git a/src/main/java/net/bramp/ffmpeg/FFmpegUtils.java b/src/main/java/net/bramp/ffmpeg/FFmpegUtils.java
@@ -76,10 +76,15 @@ public static String toTimecode(long duration, TimeUnit units) {
    * format "hour:minute:second", where second can be a decimal number.
    *
    * @param time the timecode to parse.
-   * @return the number of nanoseconds.
+   * @return the number of nanoseconds or -1 if time is 'N/A'
    */
   public static long fromTimecode(String time) {
     checkNotEmpty(time, "time must not be empty string");
+
+    if (time.equals("N/A")) {
+      return -1;
+    }
+
     Matcher m = TIME_REGEX.matcher(time);
     if (!m.find()) {
       throw new IllegalArgumentException("invalid time '" + time + "'");
@@ -99,6 +104,8 @@ public static long fromTimecode(String time) {
    * @return the bitrate in bits per second or -1 if bitrate is 'N/A'
    */
   public static long parseBitrate(String bitrate) {
+    checkNotEmpty(bitrate, "bitrate must not be empty string");
+
     if ("N/A".equals(bitrate)) {
       return -1;
     }

diff --git a/src/test/java/net/bramp/ffmpeg/FFmpegTest.java b/src/test/java/net/bramp/ffmpeg/FFmpegTest.java
@@ -7,6 +7,8 @@
 
 import java.io.IOException;
 import java.util.List;
+
+import com.google.common.collect.Lists;
 import net.bramp.ffmpeg.fixtures.Codecs;
 import net.bramp.ffmpeg.fixtures.Filters;
 import net.bramp.ffmpeg.fixtures.Formats;
@@ -35,6 +37,8 @@ public void before() throws IOException {
     when(runFunc.run(argThatHasItem("-codecs"))).thenAnswer(new NewProcessAnswer("ffmpeg-codecs"));
     when(runFunc.run(argThatHasItem("-pix_fmts")))
         .thenAnswer(new NewProcessAnswer("ffmpeg-pix_fmts"));
+    when(runFunc.run(argThatHasItem("toto.mp4")))
+        .thenAnswer(new NewProcessAnswer("ffmpeg-version", "ffmpeg-no-such-file"));
     when(runFunc.run(argThatHasItem("-filters")))
         .thenAnswer(new NewProcessAnswer("ffmpeg-filters"));
 
@@ -72,6 +76,21 @@ public void testFormats() throws IOException {
     verify(runFunc, times(1)).run(argThatHasItem("-formats"));
   }
 
+  @Test
+  public void testReadProcessStreams() throws IOException {
+    // process input stream
+    Appendable processInputStream = mock(Appendable.class);
+    ffmpeg.setProcessOutputStream(processInputStream);
+    // process error stream
+    Appendable processErrStream = mock(Appendable.class);
+    ffmpeg.setProcessErrorStream(processErrStream);
+    // run ffmpeg with non existing file
+    ffmpeg.run(Lists.newArrayList("-i", "toto.mp4"));
+    // check calls to Appendables
+    verify(processInputStream, times(1)).append(any(CharSequence.class));
+    verify(processErrStream, times(1)).append(any(CharSequence.class));
+  }
+
   @Test
   public void testPixelFormat() throws IOException {
     // Run twice, the second should be cached

diff --git a/src/test/java/net/bramp/ffmpeg/fixtures/Progresses.java b/src/test/java/net/bramp/ffmpeg/fixtures/Progresses.java
@@ -10,12 +10,20 @@ private Progresses() {
   }
 
   public static final ImmutableList<String> allFiles =
-      ImmutableList.of("ffmpeg-progress-0", "ffmpeg-progress-1", "ffmpeg-progress-2");
+      ImmutableList.of(
+              "ffmpeg-progress-0",
+              "ffmpeg-progress-1",
+              "ffmpeg-progress-2");
+
+  public static final ImmutableList<String> naProgressFile = ImmutableList.of("ffmpeg-progress-na");
 
   public static final ImmutableList<Progress> allProgresses =
       ImmutableList.of(
           new Progress(5, 0.0f, 800, 48, 512000000, 0, 0, 1.01f, Progress.Status.CONTINUE),
           new Progress(118, 23.4f, -1, -1, 5034667000L, 0, 0, -1, Progress.Status.CONTINUE),
-          new Progress(
-              132, 23.1f, 1935500, 1285168, 5312000000L, 0, 0, 0.929f, Progress.Status.END));
+          new Progress(132, 23.1f, 1935500, 1285168, 5312000000L, 0, 0, 0.929f, Progress.Status.END));
+
+  public static final ImmutableList<Progress> naProgresses = ImmutableList.of(
+          new Progress(0, 0.0f, -1, -1, -1, 0, 0, -1, Progress.Status.END)
+  );
 }
diff --git a/src/test/java/net/bramp/ffmpeg/lang/NewProcessAnswer.java b/src/test/java/net/bramp/ffmpeg/lang/NewProcessAnswer.java
@@ -7,12 +7,21 @@
 public class NewProcessAnswer implements Answer<Process> {
   final String resource;
 
+  final String errResource;
+
   public NewProcessAnswer(String resource) {
+    this(resource, null);
+  }
+
+  public NewProcessAnswer(String resource, String errResource) {
     this.resource = resource;
+    this.errResource = errResource;
   }
 
   @Override
   public Process answer(InvocationOnMock invocationOnMock) throws Throwable {
-    return new MockProcess(Helper.loadResource(resource));
+    return errResource == null
+        ? new MockProcess(Helper.loadResource(resource))
+        : new MockProcess(null, Helper.loadResource(resource), Helper.loadResource(errResource));
   }
 }
diff --git a/src/test/java/net/bramp/ffmpeg/progress/StreamProgressParserTest.java b/src/test/java/net/bramp/ffmpeg/progress/StreamProgressParserTest.java
@@ -6,7 +6,6 @@
 
 import java.io.IOException;
 import java.io.InputStream;
-import java.util.List;
 import net.bramp.ffmpeg.fixtures.Progresses;
 import org.junit.Test;
 
@@ -23,6 +22,18 @@ public void testNormal() throws IOException {
     InputStream inputStream = combineResource(Progresses.allFiles);
     parser.processStream(inputStream);
 
-    assertThat(listener.progesses, equalTo((List<Progress>) Progresses.allProgresses));
+    assertThat(listener.progesses, equalTo(Progresses.allProgresses));
+  }
+
+  @Test
+  public void testNaProgressPackets() throws IOException {
+    listener.reset();
+
+    StreamProgressParser parser = new StreamProgressParser(listener);
+
+    InputStream inputStream = combineResource(Progresses.naProgressFile);
+    parser.processStream(inputStream);
+
+    assertThat(listener.progesses, equalTo(Progresses.naProgresses));
   }
 }