From 02ceae73d5592aadfaba173c8360c6903e80318e Mon Sep 17 00:00:00 2001 From: Dylan De Faoite Date: Sat, 25 Jul 2026 13:44:34 +0100 Subject: [PATCH] fix incorrect hls segment handling --- .../ddf/vodsystem/services/MediaService.java | 2 +- .../services/media/StreamActionsService.java | 55 ++++++++++++++----- .../media/StreamActionsServiceTest.java | 40 ++++++++------ 3 files changed, 63 insertions(+), 34 deletions(-) diff --git a/src/main/java/com/ddf/vodsystem/services/MediaService.java b/src/main/java/com/ddf/vodsystem/services/MediaService.java index 56f3ba1..195d89b 100644 --- a/src/main/java/com/ddf/vodsystem/services/MediaService.java +++ b/src/main/java/com/ddf/vodsystem/services/MediaService.java @@ -238,7 +238,7 @@ public class MediaService { return new SegmentContext(segments, trimOffset); } - private Job dispatchSaveJob(Job job, SegmentContext ctx, float duration, Path outputFile, Runnable onSuccess) { + private Job dispatchSaveJob(Job job, SegmentContext ctx, float duration, Path outputFile, Runnable onSuccess) throws IOException { job.setState(JobState.PROCESSING); streamActionsService.saveSection(ctx.segments(), ctx.trimOffset(), duration, outputFile, job.getProgressTracker()) .thenRun(() -> { 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 532b55a..2c81c6a 100644 --- a/src/main/java/com/ddf/vodsystem/services/media/StreamActionsService.java +++ b/src/main/java/com/ddf/vodsystem/services/media/StreamActionsService.java @@ -11,6 +11,7 @@ import java.io.IOException; import java.nio.file.Files; import java.nio.file.Path; import java.time.Instant; +import java.util.ArrayList; import java.util.Comparator; import java.util.List; import java.util.concurrent.CompletableFuture; @@ -31,8 +32,6 @@ public class StreamActionsService { private static final Logger logger = LoggerFactory.getLogger(StreamActionsService.class); private final CommandRunner commandRunner; - private static final int HLS_SEGMENT_LENGTH = 3; - public StreamActionsService(CommandRunner commandRunner) { this.commandRunner = commandRunner; } @@ -65,21 +64,33 @@ public class StreamActionsService { float duration, Path outputFile, ProgressTracker progressTracker - ) { + ) throws IOException { + if (segments.isEmpty()) { + return CompletableFuture.failedFuture(new IllegalArgumentException("Segment list cannot be empty")); + } + + Path firstSegment = segments.getFirst(); + Path trimmedFirst = Files.createTempFile("trimmed-first-", ".ts"); try { - String concatInput = segments.stream() + // First segment must be trimmed manually due to keyframe issues with the concat protocol + List trimCommand = List.of( + "ffmpeg", "-y", + "-ss", String.valueOf(trimOffset), + "-i", firstSegment.toAbsolutePath().toString(), + "-c", "copy", + trimmedFirst.toAbsolutePath().toString() + ); + commandRunner.run(trimCommand); + + String concatInput = Stream.concat(Stream.of(trimmedFirst), segments.stream().skip(1)) .map(p -> p.toAbsolutePath().toString()) .collect(Collectors.joining("|")); - if (concatInput.isEmpty()) { - throw new IllegalStateException("Must have at least one segment"); - } - List command = List.of( "ffmpeg", "-y", - "-i", "concat:" + concatInput, "-ss", String.valueOf(trimOffset), + "-i", "concat:" + concatInput, "-t", String.valueOf(duration), "-c", "copy", outputFile.toAbsolutePath().toString() @@ -101,6 +112,8 @@ public class StreamActionsService { } catch (RuntimeException e) { logger.error("Failed to save section to '{}': {}", outputFile, e.toString()); return CompletableFuture.failedFuture(e); + } finally { + Files.deleteIfExists(trimmedFirst); } } @@ -112,17 +125,29 @@ public class StreamActionsService { throw new IllegalArgumentException("Start time must be before end time"); } + List results = new ArrayList<>(); + try (Stream files = Files.list(streamDirectory)) { - return files + List streamFiles = files .filter(p -> p.getFileName().toString().endsWith(".ts")) .filter(Files::isRegularFile) - .filter(p -> { - long segMs = parseTimestampMs(p); - return segMs < endMs && (segMs + HLS_SEGMENT_LENGTH * 1000) > startMs; - }) .sorted(Comparator.comparingLong(this::parseTimestampMs)) - .collect(Collectors.toList()); + .toList(); + + for (int i = 0; i < streamFiles.size(); i++) { + Path streamFile = streamFiles.get(i); + long segmentStart = parseTimestampMs(streamFile); + long segmentEnd = (i + 1 < streamFiles.size()) + ? parseTimestampMs(streamFiles.get(i + 1)) + : Long.MAX_VALUE; + + if (segmentStart < endMs && segmentEnd > startMs) { + results.add(streamFile); + } + } } + + return results; } public long parseTimestampMs(Path path) { 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 16baba1..cb0d563 100644 --- a/src/test/java/com/ddf/vodsystem/services/media/StreamActionsServiceTest.java +++ b/src/test/java/com/ddf/vodsystem/services/media/StreamActionsServiceTest.java @@ -124,13 +124,13 @@ class StreamActionsServiceTest { @Test void getSegmentsInRange_segmentEndsExactlyAtRangeStart_isExcluded() throws IOException { - // "0.ts" spans [0, 3000)ms. A request starting exactly at 3000ms should not - // pull in a segment that ended the instant before. createSegment("0.ts"); + createSegment("3.ts"); List result = service.getSegmentsInRange(tempDir, Instant.ofEpochMilli(3000), Instant.ofEpochMilli(10_000)); - assertThat(result).isEmpty(); + assertThat(result).extracting(p -> p.getFileName().toString()) + .containsExactly("3.ts"); } @Test @@ -170,16 +170,6 @@ class StreamActionsServiceTest { } - @Test - void getSegmentsInRange_segmentsWhollyOutsideRange_areExcluded() throws IOException { - createSegment("0.ts"); // [0, 3000) - createSegment("100.ts"); // [100000, 103000) - - List result = service.getSegmentsInRange(tempDir, Instant.ofEpochMilli(50_000), Instant.ofEpochMilli(60_000)); - - assertThat(result).isEmpty(); - } - @Test void getSegmentsInRange_segmentsOutOfOrder_returnedInOrder() throws IOException { createSegment("20.ts"); @@ -219,6 +209,20 @@ class StreamActionsServiceTest { .containsExactly("10.ts", "20.ts", "30.ts"); } + @Test + void getSegmentsInRange_gapInStream_returnsTheGap() throws IOException { + createSegment("20.ts"); + createSegment("30.ts"); + // user stops streaming + createSegment("100.ts"); + createSegment("110.ts"); + + List result = service.getSegmentsInRange(tempDir, Instant.ofEpochMilli(70_000), Instant.ofEpochMilli(111_000)); + + assertThat(result).extracting(p -> p.getFileName().toString()) + .containsExactly("30.ts", "100.ts", "110.ts"); + } + // --------------------------------------------------------------- // parseTimestampMs: unit-based conversion // --------------------------------------------------------------- @@ -275,6 +279,7 @@ class StreamActionsServiceTest { ProgressTracker progress = new ProgressTracker(); CommandOutput commandOutput = new CommandOutput(); + when(commandRunner.run(anyList())).thenReturn(commandOutput); when(commandRunner.run(anyList(), any())).thenReturn(commandOutput); var future = service.saveSection(List.of(segmentA, segmentB), 5.0f, 12.0f, output, progress); @@ -287,8 +292,6 @@ class StreamActionsServiceTest { Mockito.verify(commandRunner).run(captor.capture(), any()); List command = captor.getValue(); - assertThat(command).containsSequence("-i", - "concat:" + segmentA.toAbsolutePath() + "|" + segmentB.toAbsolutePath()); assertThat(command).containsSequence("-ss", "5.0"); assertThat(command).containsSequence("-t", "12.0"); assertThat(command).containsSequence("-c", "copy"); @@ -300,6 +303,7 @@ class StreamActionsServiceTest { Path output = Path.of("/tmp/out/clip.mp4"); ProgressTracker progress = new ProgressTracker(); + when(commandRunner.run(anyList())).thenReturn(new CommandOutput()); when(commandRunner.run(anyList(), any())).thenThrow(new IOException("ffmpeg boom")); var future = service.saveSection(List.of(Path.of("/tmp/a.ts")), 0f, 10f, output, progress); @@ -311,7 +315,7 @@ class StreamActionsServiceTest { } @Test - void saveSection_emptySegmentList_completesExceptionallyWithIllegalStateException() { + void saveSection_emptySegmentList_completesExceptionallyWithIllegalArgumentException() throws IOException { Path output = Path.of("/tmp/out/clip.mp4"); ProgressTracker progress = new ProgressTracker(); @@ -319,12 +323,12 @@ class StreamActionsServiceTest { assertThatThrownBy(future::get) .isInstanceOf(ExecutionException.class) - .hasCauseInstanceOf(IllegalStateException.class); + .hasCauseInstanceOf(IllegalArgumentException.class); assertThat(progress.isComplete()).isFalse(); } @Test - void saveSection_nullOutputFile_completesExceptionally() { + void saveSection_nullOutputFile_completesExceptionally() throws IOException { ProgressTracker progress = new ProgressTracker(); var future = service.saveSection(List.of(Path.of("/tmp/a.ts")), 0f, 10f, null, progress);