fix incorrect hls segment handling
All checks were successful
CI / Maven unit tests (push) Successful in 15s
CI / Build and push images (push) Has been skipped
CI / Deploy to server (push) Has been skipped

This commit is contained in:
2026-07-25 13:44:34 +01:00
parent ae8f2f33a7
commit 02ceae73d5
3 changed files with 63 additions and 34 deletions

View File

@@ -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(() -> {

View File

@@ -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<String> 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<String> 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<Path> results = new ArrayList<>();
try (Stream<Path> files = Files.list(streamDirectory)) {
return files
List<Path> 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) {

View File

@@ -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<Path> 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<Path> 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<Path> 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<String> 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);