From 2408263988034d02d43ff44226c331680f703e59 Mon Sep 17 00:00:00 2001 From: Dylan De Faoite Date: Sun, 12 Jul 2026 18:49:20 +0100 Subject: [PATCH] fix(media): propagate async FFmpeg results and failures --- .../ddf/vodsystem/services/ClipService.java | 13 +++---- .../ddf/vodsystem/services/MediaService.java | 3 +- .../ddf/vodsystem/services/VodService.java | 13 +++---- .../services/media/CompressionService.java | 27 ++++++++----- .../services/media/MetadataService.java | 7 ++-- .../services/media/RemuxService.java | 38 ++++++++++++------- .../services/media/StreamActionsService.java | 27 ++++++++----- .../services/media/ThumbnailService.java | 28 +++++++++----- .../services/media/MetadataServiceTest.java | 17 +++++---- .../media/StreamActionsServiceTest.java | 20 ++++++---- 10 files changed, 116 insertions(+), 77 deletions(-) diff --git a/src/main/java/com/ddf/vodsystem/services/ClipService.java b/src/main/java/com/ddf/vodsystem/services/ClipService.java index e0dfb43..763e382 100644 --- a/src/main/java/com/ddf/vodsystem/services/ClipService.java +++ b/src/main/java/com/ddf/vodsystem/services/ClipService.java @@ -222,14 +222,11 @@ public class ClipService { } // Thumbnail generation can fail with error propagation - try { - thumbnailService.createThumbnail(newClipFile, thumbnailFile, 0.0f); - } catch (InterruptedException e) { - logger.error("Thumbnail generation interrupted for user: {}", user.getId(), e); - Thread.currentThread().interrupt(); - } catch (IOException e) { - logger.error("Error generating thumbnail for user: {}", user.getId(), e); - } + thumbnailService.createThumbnail(newClipFile, thumbnailFile, 0.0f) + .exceptionally(ex -> { + logger.error("Thumbnail job for user {} failed: {}", user.getId(), ex.toString()); + return null; + }); clipMetadata.setTitle(title); clipMetadata.setDescription(description); diff --git a/src/main/java/com/ddf/vodsystem/services/MediaService.java b/src/main/java/com/ddf/vodsystem/services/MediaService.java index c451822..2c6b43b 100644 --- a/src/main/java/com/ddf/vodsystem/services/MediaService.java +++ b/src/main/java/com/ddf/vodsystem/services/MediaService.java @@ -59,9 +59,8 @@ public class MediaService { * @return the {@link Job} tracking the compression; state transitions to * {@link JobState#SUCCEEDED} or {@link JobState#FAILED} asynchronously * @throws IOException if saving the upload to the temporary directory fails - * @throws InterruptedException if the current thread is interrupted while setting up the job */ - public Job compress(MultipartFile file, ClipOptions clipOptions) throws IOException, InterruptedException { + public Job compress(MultipartFile file, ClipOptions clipOptions) throws IOException { Job job = jobRegistryService.generateJob(); Optional user = userService.getLoggedInUser(); diff --git a/src/main/java/com/ddf/vodsystem/services/VodService.java b/src/main/java/com/ddf/vodsystem/services/VodService.java index ff51c87..9e19947 100644 --- a/src/main/java/com/ddf/vodsystem/services/VodService.java +++ b/src/main/java/com/ddf/vodsystem/services/VodService.java @@ -201,14 +201,11 @@ public class VodService { throw new FFMPEGException("Error retrieving video metadata for vod: " + e.getMessage()); } - try { - thumbnailService.createThumbnail(newVodFile, thumbnailFile, 0.0f); - } catch (InterruptedException e) { - logger.error("Thumbnail generation interrupted for user: {}", user.getId(), e); - Thread.currentThread().interrupt(); - } catch (IOException e) { - logger.error("Error generating thumbnail for user: {}", user.getId(), e); - } + thumbnailService.createThumbnail(newVodFile, thumbnailFile, 0.0f) + .exceptionally(ex -> { + logger.error("Thumbnail job for user {} failed: {}", user.getId(), ex.toString()); + return null; + }); vodMetadata.setTitle(title); vodMetadata.setDescription(description); diff --git a/src/main/java/com/ddf/vodsystem/services/media/CompressionService.java b/src/main/java/com/ddf/vodsystem/services/media/CompressionService.java index 84bd355..b1299c0 100644 --- a/src/main/java/com/ddf/vodsystem/services/media/CompressionService.java +++ b/src/main/java/com/ddf/vodsystem/services/media/CompressionService.java @@ -32,18 +32,27 @@ public class CompressionService { Path outputFile, ClipOptions clipOptions, ProgressTracker progress - ) throws IOException, InterruptedException { + ) { logger.info("Compressing video from {} to {}", inputFile.toAbsolutePath(), outputFile.toAbsolutePath()); - List command = buildCommand( - inputFile, - outputFile, - clipOptions - ); - CommandOutput result = commandRunner.run(command, line -> commandRunner.setProgress(line, progress, clipOptions.getDuration())); - progress.markComplete(); + try { + List command = buildCommand( + inputFile, + outputFile, + clipOptions + ); + CommandOutput result = commandRunner.run(command, line -> commandRunner.setProgress(line, progress, clipOptions.getDuration())); + progress.markComplete(); - return CompletableFuture.completedFuture(result); + return CompletableFuture.completedFuture(result); + } catch (IOException e) { + logger.error("IO error on compress call: {}", e.toString()); + return CompletableFuture.failedFuture(e); + } catch (InterruptedException e) { + logger.error("Thread error on compress call: {}", e.toString()); + Thread.currentThread().interrupt(); + return CompletableFuture.failedFuture(e); + } } private List buildFilters(Float fps, Integer width, Integer height) { diff --git a/src/main/java/com/ddf/vodsystem/services/media/MetadataService.java b/src/main/java/com/ddf/vodsystem/services/media/MetadataService.java index 602cb68..08bf15b 100644 --- a/src/main/java/com/ddf/vodsystem/services/media/MetadataService.java +++ b/src/main/java/com/ddf/vodsystem/services/media/MetadataService.java @@ -14,7 +14,6 @@ import java.io.IOException; import java.nio.file.Path; import java.util.List; import java.util.concurrent.CompletableFuture; -import java.util.concurrent.Future; @Service public class MetadataService { @@ -28,7 +27,7 @@ public class MetadataService { } @Async("ffmpegTaskExecutor") - public Future getVideoMetadata(Path file) { + public CompletableFuture getVideoMetadata(Path file) { logger.info("Getting metadata for file {}", file); List command = List.of( @@ -53,10 +52,10 @@ public class MetadataService { JsonNode node = mapper.readTree(outputBuilder.toString()); return CompletableFuture.completedFuture(parseVideoMetadata(node)); } catch (IOException e) { - throw new FFMPEGException("Error while getting video metadata: " + e); + return CompletableFuture.failedFuture(e); } catch (InterruptedException e) { Thread.currentThread().interrupt(); - throw new FFMPEGException("Error while getting video metadata: " + e); + return CompletableFuture.failedFuture(e); } } diff --git a/src/main/java/com/ddf/vodsystem/services/media/RemuxService.java b/src/main/java/com/ddf/vodsystem/services/media/RemuxService.java index 3fffafb..38ac85c 100644 --- a/src/main/java/com/ddf/vodsystem/services/media/RemuxService.java +++ b/src/main/java/com/ddf/vodsystem/services/media/RemuxService.java @@ -2,6 +2,8 @@ package com.ddf.vodsystem.services.media; import com.ddf.vodsystem.dto.CommandOutput; import com.ddf.vodsystem.dto.ProgressTracker; +import org.slf4j.Logger; +import org.slf4j.LoggerFactory; import org.springframework.scheduling.annotation.Async; import org.springframework.stereotype.Service; @@ -12,6 +14,7 @@ import java.util.concurrent.CompletableFuture; @Service public class RemuxService { + private static final Logger logger = LoggerFactory.getLogger(RemuxService.class); private final CommandRunner commandRunner; public RemuxService(CommandRunner commandRunner) { @@ -23,19 +26,28 @@ public class RemuxService { File outputFile, ProgressTracker remuxProgress, float length - ) throws IOException, InterruptedException { - List command = List.of( - "ffmpeg", - "-progress", "pipe:1", - "-y", - "-i", inputFile.getAbsolutePath(), - "-c:v", "h264", - "-c:a", "aac", - "-f", "mp4", - outputFile.getAbsolutePath() - ); + ) { + try { + List command = List.of( + "ffmpeg", + "-progress", "pipe:1", + "-y", + "-i", inputFile.getAbsolutePath(), + "-c:v", "h264", + "-c:a", "aac", + "-f", "mp4", + outputFile.getAbsolutePath() + ); - return CompletableFuture.completedFuture(commandRunner.run(command, line -> - commandRunner.setProgress(line, remuxProgress, length))); + return CompletableFuture.completedFuture(commandRunner.run(command, line -> + commandRunner.setProgress(line, remuxProgress, length))); + } catch (IOException e) { + logger.error("IO error on remux call: {}", e.toString()); + return CompletableFuture.failedFuture(e); + } catch (InterruptedException e) { + logger.error("Thread error on remux call: {}", e.toString()); + Thread.currentThread().interrupt(); + return CompletableFuture.failedFuture(e); + } } } diff --git a/src/main/java/com/ddf/vodsystem/services/media/StreamActionsService.java b/src/main/java/com/ddf/vodsystem/services/media/StreamActionsService.java index d3e7160..532b55a 100644 --- a/src/main/java/com/ddf/vodsystem/services/media/StreamActionsService.java +++ b/src/main/java/com/ddf/vodsystem/services/media/StreamActionsService.java @@ -1,5 +1,6 @@ package com.ddf.vodsystem.services.media; +import com.ddf.vodsystem.dto.CommandOutput; import com.ddf.vodsystem.dto.ProgressTracker; import org.slf4j.Logger; import org.slf4j.LoggerFactory; @@ -54,12 +55,11 @@ public class StreamActionsService { * to extract * @param outputFile the destination file the trimmed section will be * written to; overwritten if it already exists - * @return a {@link CompletableFuture} that completes with - * {@code outputFile} on success, or completes exceptionally on - * failure + * @return a {@link CompletableFuture} that completes with the ffmpeg + * command output on success, or completes exceptionally on failure */ @Async("ffmpegTaskExecutor") - public CompletableFuture saveSection( + public CompletableFuture saveSection( List segments, float trimOffset, float duration, @@ -87,12 +87,19 @@ public class StreamActionsService { logger.info("Saving section with ({} segments) to '{}'", segments.size(), outputFile); - commandRunner.run(command, line -> commandRunner.setProgress(line, progressTracker, duration)); - progressTracker.markComplete(); + CommandOutput output = commandRunner.run(command, line -> commandRunner.setProgress(line, progressTracker, duration)); - return CompletableFuture.completedFuture(outputFile); - } catch (Exception e) { - logger.error("Failed to save section to '{}'", outputFile, e); + progressTracker.markComplete(); + return CompletableFuture.completedFuture(output); + } catch (IOException e) { + logger.error("IO error on saveSection call: {}", e.toString()); + return CompletableFuture.failedFuture(e); + } catch (InterruptedException e) { + logger.error("Thread error on saveSection call: {}", e.toString()); + Thread.currentThread().interrupt(); + return CompletableFuture.failedFuture(e); + } catch (RuntimeException e) { + logger.error("Failed to save section to '{}': {}", outputFile, e.toString()); return CompletableFuture.failedFuture(e); } } @@ -130,4 +137,4 @@ public class StreamActionsService { // nginx hls_fragment_naming system uses seconds; convert to ms return value < 1_000_000_000_000L ? value * 1000L : value; } -} \ No newline at end of file +} diff --git a/src/main/java/com/ddf/vodsystem/services/media/ThumbnailService.java b/src/main/java/com/ddf/vodsystem/services/media/ThumbnailService.java index 7df5d18..70fa517 100644 --- a/src/main/java/com/ddf/vodsystem/services/media/ThumbnailService.java +++ b/src/main/java/com/ddf/vodsystem/services/media/ThumbnailService.java @@ -1,5 +1,6 @@ package com.ddf.vodsystem.services.media; +import com.ddf.vodsystem.dto.CommandOutput; import org.slf4j.Logger; import org.slf4j.LoggerFactory; import org.springframework.scheduling.annotation.Async; @@ -8,6 +9,7 @@ import org.springframework.stereotype.Service; import java.io.IOException; import java.nio.file.Path; import java.util.List; +import java.util.concurrent.CompletableFuture; @Service public class ThumbnailService { @@ -19,17 +21,25 @@ public class ThumbnailService { } @Async("ffmpegTaskExecutor") - public void createThumbnail(Path inputFile, Path outputFile, Float timeInVideo) throws IOException, InterruptedException { + public CompletableFuture createThumbnail(Path inputFile, Path outputFile, Float timeInVideo) { logger.info("Creating thumbnail at {} seconds", timeInVideo); - List command = List.of( - "ffmpeg", - "-ss", timeInVideo.toString(), - "-i", inputFile.toAbsolutePath().toString(), - "-frames:v", "1", - outputFile.toAbsolutePath().toString() - ); + try { + List command = List.of( + "ffmpeg", + "-ss", timeInVideo.toString(), + "-i", inputFile.toAbsolutePath().toString(), + "-frames:v", "1", + outputFile.toAbsolutePath().toString() + ); - commandRunner.run(command); + CommandOutput output = commandRunner.run(command); + return CompletableFuture.completedFuture(output); + } catch (IOException e) { + return CompletableFuture.failedFuture(e); + } catch (InterruptedException e) { + Thread.currentThread().interrupt(); + return CompletableFuture.failedFuture(e); + } } } diff --git a/src/test/java/com/ddf/vodsystem/services/media/MetadataServiceTest.java b/src/test/java/com/ddf/vodsystem/services/media/MetadataServiceTest.java index 42084cb..0e4d241 100644 --- a/src/test/java/com/ddf/vodsystem/services/media/MetadataServiceTest.java +++ b/src/test/java/com/ddf/vodsystem/services/media/MetadataServiceTest.java @@ -53,16 +53,19 @@ class MetadataServiceTest { private ClipOptions runMetadata() throws Exception { return metadataService.getVideoMetadata(INPUT).get(); } - - private void assertMetadataFailsWith(String messageFragment) { + private void assertMetadataFailsWith(Class exceptionType, String messageFragment) { assertThatThrownBy(() -> metadataService.getVideoMetadata(INPUT).get()) .satisfies(thrown -> { Throwable cause = thrown instanceof ExecutionException ? thrown.getCause() : thrown; - assertThat(cause).isInstanceOf(FFMPEGException.class); + assertThat(cause).isInstanceOf(exceptionType); assertThat(cause.getMessage()).contains(messageFragment); }); } + private void assertMetadataFailsWith(String messageFragment) { + assertMetadataFailsWith(FFMPEGException.class, messageFragment); + } + // --------------------------------------------------------------- // Happy path / parsing behavior // --------------------------------------------------------------- @@ -246,10 +249,10 @@ class MetadataServiceTest { } @Test - void getVideoMetadata_commandThrowsIOException_throwsFFMPEGException() throws Exception { + void getVideoMetadata_commandThrowsIOException_completesExceptionally() throws Exception { when(commandRunner.run(anyList())).thenThrow(new IOException("ffprobe boom")); - assertMetadataFailsWith("Error while getting video metadata"); + assertMetadataFailsWith(IOException.class, "ffprobe boom"); } @Test @@ -291,7 +294,7 @@ class MetadataServiceTest { void getVideoMetadata_plainIOException_doesNotSetThreadInterruptFlag() throws Exception { when(commandRunner.run(anyList())).thenThrow(new IOException("ffprobe boom")); - assertMetadataFailsWith("Error while getting video metadata"); + assertMetadataFailsWith(IOException.class, "ffprobe boom"); assertThat(Thread.currentThread().isInterrupted()) .as("a plain IOException must not leave the calling thread's interrupt flag set") @@ -302,7 +305,7 @@ class MetadataServiceTest { void getVideoMetadata_interruptedException_setsThreadInterruptFlag() throws Exception { when(commandRunner.run(anyList())).thenThrow(new InterruptedException("interrupted")); - assertMetadataFailsWith("Error while getting video metadata"); + assertMetadataFailsWith(InterruptedException.class, "interrupted"); assertThat(Thread.currentThread().isInterrupted()).isTrue(); } diff --git a/src/test/java/com/ddf/vodsystem/services/media/StreamActionsServiceTest.java b/src/test/java/com/ddf/vodsystem/services/media/StreamActionsServiceTest.java index a720cfc..16baba1 100644 --- a/src/test/java/com/ddf/vodsystem/services/media/StreamActionsServiceTest.java +++ b/src/test/java/com/ddf/vodsystem/services/media/StreamActionsServiceTest.java @@ -268,17 +268,18 @@ class StreamActionsServiceTest { // --------------------------------------------------------------- @Test - void saveSection_success_buildsExpectedCommandAndCompletesWithOutputFile() throws Exception { + void saveSection_success_buildsExpectedCommandAndCompletesWithCommandOutput() throws Exception { Path segmentA = Path.of("/tmp/streams/a.ts"); Path segmentB = Path.of("/tmp/streams/b.ts"); Path output = Path.of("/tmp/out/clip.mp4"); ProgressTracker progress = new ProgressTracker(); + CommandOutput commandOutput = new CommandOutput(); - when(commandRunner.run(anyList(), any())).thenReturn(new CommandOutput()); + when(commandRunner.run(anyList(), any())).thenReturn(commandOutput); var future = service.saveSection(List.of(segmentA, segmentB), 5.0f, 12.0f, output, progress); - assertThat(future.get()).isEqualTo(output); + assertThat(future.get()).isSameAs(commandOutput); assertThat(progress.isComplete()).isTrue(); @SuppressWarnings("unchecked") @@ -310,14 +311,16 @@ class StreamActionsServiceTest { } @Test - void saveSection_emptySegmentList_throwsIllegalStateException() { + void saveSection_emptySegmentList_completesExceptionallyWithIllegalStateException() { Path output = Path.of("/tmp/out/clip.mp4"); ProgressTracker progress = new ProgressTracker(); var future = service.saveSection(List.of(), 0f, 10f, output, progress); - assertThatThrownBy(future::get).isInstanceOf(ExecutionException.class); - + assertThatThrownBy(future::get) + .isInstanceOf(ExecutionException.class) + .hasCauseInstanceOf(IllegalStateException.class); + assertThat(progress.isComplete()).isFalse(); } @Test @@ -326,6 +329,9 @@ class StreamActionsServiceTest { var future = service.saveSection(List.of(Path.of("/tmp/a.ts")), 0f, 10f, null, progress); - assertThatThrownBy(future::get).isInstanceOf(ExecutionException.class); + assertThatThrownBy(future::get) + .isInstanceOf(ExecutionException.class) + .hasCauseInstanceOf(NullPointerException.class); + assertThat(progress.isComplete()).isFalse(); } }