diff --git a/src/seconv/Core/SubtitleConverter.cs b/src/seconv/Core/SubtitleConverter.cs index 96d9ebb7379..d5dfa6141da 100644 --- a/src/seconv/Core/SubtitleConverter.cs +++ b/src/seconv/Core/SubtitleConverter.cs @@ -252,24 +252,6 @@ private async Task ConvertVobBatchAsync(List vobFiles, outputBase = Path.Combine(outputFolder, stem + ".sub"); } - // Overwrite check is best-effort against the base path. Multi-stream DVDs land - // additional outputs at ..sub which we can't predict without parsing — - // those existing files will be overwritten silently. Acceptable for now since - // multi-stream DVDs are rare and the user opted into the batch by passing all VOBs. - if (!options.Overwrite) - { - var pair = new[] { outputBase, Path.ChangeExtension(outputBase, ".idx") }; - foreach (var p in pair) - { - if (File.Exists(p)) - { - result.Errors.Add($"Output file already exists: {p}. Pass --overwrite to replace it."); - result.FailedFiles = vobFiles.Count; - return result; - } - } - } - if (!options.Quiet) { var label = vobFiles.Count == 1 @@ -283,7 +265,7 @@ private async Task ConvertVobBatchAsync(List vobFiles, // IsPal — there's no single reliable auto-detect from VOB alone (would need // IFO parsing). Default to PAL to match the GUI's batch converter. Future // work: add --vob-pal/--vob-ntsc and/or read VIDEO_TS.IFO. - var outputs = VobSubExtractor.Extract(vobFiles, outputBase, isPal: true); + var outputs = VobSubExtractor.Extract(vobFiles, outputBase, isPal: true, overwrite: options.Overwrite); result.SuccessfulFiles = vobFiles.Count; // Report the first stream's output path against each input VOB. With multiple // streams there's no clean 1:1 mapping back to inputs, but the OutputFile slot diff --git a/src/seconv/Core/VobSubExtractor.cs b/src/seconv/Core/VobSubExtractor.cs index f608f2d1f9c..45bd158e4f5 100644 --- a/src/seconv/Core/VobSubExtractor.cs +++ b/src/seconv/Core/VobSubExtractor.cs @@ -31,7 +31,7 @@ public sealed record StreamOutput(string Path, int StreamId, int Written); /// movie.0.sub, movie.1.sub, …) and the matching .idx is /// written alongside each one. /// - public static IReadOnlyList Extract(IReadOnlyList vobFiles, string subOutputPath, bool isPal) + public static IReadOnlyList Extract(IReadOnlyList vobFiles, string subOutputPath, bool isPal, bool overwrite = true) { if (vobFiles.Count == 0) { @@ -72,15 +72,15 @@ public static IReadOnlyList Extract(IReadOnlyList vobFiles .OrderBy(g => g.Key) .ToList(); + var outputPaths = BuildOutputPaths(subOutputPath, streams.Count); + EnsureOutputFilesCanBeWritten(outputPaths, overwrite); + var outputs = new List(streams.Count); for (var i = 0; i < streams.Count; i++) { var streamId = streams[i].Key; var streamPacks = streams[i].OrderBy(p => p.StartTime.Ticks).ToList(); - - var outputPath = streams.Count == 1 - ? subOutputPath - : InsertStreamIndex(subOutputPath, i); + var outputPath = outputPaths[i]; var written = WriteOneStream(streamPacks, outputPath, isPal, streamId); outputs.Add(new StreamOutput(outputPath, streamId, written)); @@ -100,6 +100,47 @@ private static string InsertStreamIndex(string subOutputPath, int index) return Path.Combine(dir, $"{stem}.{index}.sub"); } + internal static IReadOnlyList BuildOutputPaths(string subOutputPath, int streamCount) + { + if (streamCount <= 0) + { + return []; + } + + if (streamCount == 1) + { + return [subOutputPath]; + } + + var outputPaths = new List(streamCount); + for (var i = 0; i < streamCount; i++) + { + outputPaths.Add(InsertStreamIndex(subOutputPath, i)); + } + + return outputPaths; + } + + internal static void EnsureOutputFilesCanBeWritten(IReadOnlyList outputPaths, bool overwrite) + { + if (overwrite) + { + return; + } + + foreach (var outputPath in outputPaths) + { + var idxPath = Path.ChangeExtension(outputPath, ".idx"); + foreach (var path in new[] { outputPath, idxPath }) + { + if (File.Exists(path)) + { + throw new IOException($"Output file already exists: {path}. Pass --overwrite to replace it."); + } + } + } + } + private static int WriteOneStream(IReadOnlyList packs, string outputPath, bool isPal, int streamId) { var screenWidth = 720; diff --git a/tests/seconv/Core/VobSubExtractorTest.cs b/tests/seconv/Core/VobSubExtractorTest.cs index a08ef9720cb..06507780c2e 100644 --- a/tests/seconv/Core/VobSubExtractorTest.cs +++ b/tests/seconv/Core/VobSubExtractorTest.cs @@ -79,4 +79,37 @@ public async Task ConvertAsync_VobInput_WithVobSubTarget_AttemptsExtraction() // No "input file too large" leak. Assert.DoesNotContain("too large", result.Errors[0]); } + + [Fact] + public void EnsureOutputFilesCanBeWritten_MultiStream_NoOverwrite_ProtectsNumberedOutputs() + { + var outputBase = Path.Combine(_tempRoot, "movie.sub"); + var outputPaths = VobSubExtractor.BuildOutputPaths(outputBase, 2); + Assert.Equal(Path.Combine(_tempRoot, "movie.0.sub"), outputPaths[0]); + Assert.Equal(Path.Combine(_tempRoot, "movie.1.sub"), outputPaths[1]); + + var existing = Path.ChangeExtension(outputPaths[1], ".idx"); + File.WriteAllText(existing, "keep-me"); + + var ex = Assert.Throws(() => + VobSubExtractor.EnsureOutputFilesCanBeWritten(outputPaths, overwrite: false)); + + Assert.Contains(existing, ex.Message); + Assert.Equal("keep-me", File.ReadAllText(existing)); + Assert.False(File.Exists(outputPaths[0])); + Assert.False(File.Exists(outputPaths[1])); + } + + [Fact] + public void EnsureOutputFilesCanBeWritten_MultiStream_DoesNotBlockUnusedBasePath() + { + var outputBase = Path.Combine(_tempRoot, "movie.sub"); + File.WriteAllText(outputBase, "unrelated-existing-base"); + + var outputPaths = VobSubExtractor.BuildOutputPaths(outputBase, 2); + VobSubExtractor.EnsureOutputFilesCanBeWritten(outputPaths, overwrite: false); + + Assert.Equal("unrelated-existing-base", File.ReadAllText(outputBase)); + } + }