Merge pull request #17746 from kfarnung/fix/vobsub-idx-subtitle-language
Probe .idx instead of .sub for external VobSub subtitle language detection
This commit is contained in:
@@ -44,7 +44,14 @@ namespace Emby.Naming.ExternalFiles
|
||||
}
|
||||
|
||||
var extension = Path.GetExtension(path.AsSpan());
|
||||
if (!(_type == DlnaProfileType.Subtitle && _namingOptions.SubtitleFileExtensions.Contains(extension, StringComparison.OrdinalIgnoreCase))
|
||||
|
||||
// .idx carries VobSub per-track language metadata. Recognize it here rather
|
||||
// than adding it to NamingOptions.SubtitleFileExtensions, which also gates
|
||||
// subtitle uploads/saves.
|
||||
var isVobSubIndex = _type == DlnaProfileType.Subtitle && extension.Equals(".idx", StringComparison.OrdinalIgnoreCase);
|
||||
|
||||
if (!isVobSubIndex
|
||||
&& !(_type == DlnaProfileType.Subtitle && _namingOptions.SubtitleFileExtensions.Contains(extension, StringComparison.OrdinalIgnoreCase))
|
||||
&& !(_type == DlnaProfileType.Audio && _namingOptions.AudioFileExtensions.Contains(extension, StringComparison.OrdinalIgnoreCase))
|
||||
&& !(_type == DlnaProfileType.Lyric && _namingOptions.LyricFileExtensions.Contains(extension, StringComparison.OrdinalIgnoreCase)))
|
||||
{
|
||||
|
||||
@@ -231,10 +231,28 @@ namespace MediaBrowser.Providers.MediaInfo
|
||||
return Array.Empty<ExternalPathParserResult>();
|
||||
}
|
||||
|
||||
// VobSub .sub payloads only carry per-track language metadata when read via
|
||||
// their paired .idx file, so probe the .idx instead and skip the .sub. Pairing
|
||||
// requires the same directory (ffprobe can't resolve a split pair) and an
|
||||
// ordinal comparison (ffprobe matches the .sub by exact case on case-sensitive
|
||||
// filesystems, so a looser match could suppress a .sub with no working .idx).
|
||||
// An .idx file with no paired .sub cannot be probed at all, so it is left out
|
||||
// entirely rather than surfaced (which would otherwise fail every probe and,
|
||||
// since the .idx would keep "existing" from Jellyfin's point of view, prevent
|
||||
// stale subtitle stream metadata from ever being cleared once the .sub is gone).
|
||||
HashSet<string>? pairedVobSubKeys = _type == DlnaProfileType.Subtitle
|
||||
? GetPairedVobSubKeys(files)
|
||||
: null;
|
||||
|
||||
var externalPathInfos = new List<ExternalPathParserResult>();
|
||||
ReadOnlySpan<char> prefix = video.FileNameWithoutExtension;
|
||||
foreach (var file in files)
|
||||
{
|
||||
if (IsSuppressedVobSubFile(file, pairedVobSubKeys))
|
||||
{
|
||||
continue;
|
||||
}
|
||||
|
||||
var fileNameWithoutExtension = Path.GetFileNameWithoutExtension(file.AsSpan());
|
||||
if (fileNameWithoutExtension.Length >= prefix.Length
|
||||
&& prefix.Equals(fileNameWithoutExtension[..prefix.Length], StringComparison.OrdinalIgnoreCase)
|
||||
@@ -304,6 +322,77 @@ namespace MediaBrowser.Providers.MediaInfo
|
||||
return externalPathInfos;
|
||||
}
|
||||
|
||||
/// <summary>
|
||||
/// Determines whether a candidate file is part of a VobSub .idx/.sub pair that
|
||||
/// should be resolved to only its .idx file, or an .idx file with no paired .sub
|
||||
/// that cannot be probed at all.
|
||||
/// </summary>
|
||||
/// <param name="file">The full path to the candidate file.</param>
|
||||
/// <param name="pairedVobSubKeys">The set of pairing keys with both an .idx and .sub present, or null if not applicable.</param>
|
||||
/// <returns><c>true</c> if the file should be suppressed; otherwise, <c>false</c>.</returns>
|
||||
private static bool IsSuppressedVobSubFile(string file, HashSet<string>? pairedVobSubKeys)
|
||||
{
|
||||
if (pairedVobSubKeys is null)
|
||||
{
|
||||
return false;
|
||||
}
|
||||
|
||||
var extension = Path.GetExtension(file.AsSpan());
|
||||
if (extension.Equals(".sub", StringComparison.OrdinalIgnoreCase))
|
||||
{
|
||||
// A paired .idx exists; probe it instead of the .sub payload.
|
||||
return pairedVobSubKeys.Contains(GetVobSubPairingKey(file));
|
||||
}
|
||||
|
||||
if (extension.Equals(".idx", StringComparison.OrdinalIgnoreCase))
|
||||
{
|
||||
// Without its .sub payload, the .idx cannot be probed for any data.
|
||||
return !pairedVobSubKeys.Contains(GetVobSubPairingKey(file));
|
||||
}
|
||||
|
||||
return false;
|
||||
}
|
||||
|
||||
/// <summary>
|
||||
/// Builds the set of directory+basename keys that have both an .idx and a .sub
|
||||
/// file present, in a single pass over the candidate files.
|
||||
/// </summary>
|
||||
/// <param name="files">The candidate files to search.</param>
|
||||
/// <returns>The set of pairing keys with both an .idx and .sub present.</returns>
|
||||
private static HashSet<string> GetPairedVobSubKeys(IEnumerable<string> files)
|
||||
{
|
||||
var idxKeys = new HashSet<string>(StringComparer.Ordinal);
|
||||
var subKeys = new HashSet<string>(StringComparer.Ordinal);
|
||||
foreach (var file in files)
|
||||
{
|
||||
var extension = Path.GetExtension(file.AsSpan());
|
||||
if (extension.Equals(".idx", StringComparison.OrdinalIgnoreCase))
|
||||
{
|
||||
idxKeys.Add(GetVobSubPairingKey(file));
|
||||
}
|
||||
else if (extension.Equals(".sub", StringComparison.OrdinalIgnoreCase))
|
||||
{
|
||||
subKeys.Add(GetVobSubPairingKey(file));
|
||||
}
|
||||
}
|
||||
|
||||
idxKeys.IntersectWith(subKeys);
|
||||
return idxKeys;
|
||||
}
|
||||
|
||||
/// <summary>
|
||||
/// Builds a directory+basename key used to pair a VobSub .idx file with its .sub
|
||||
/// payload only when both live in the same directory.
|
||||
/// </summary>
|
||||
/// <param name="file">The full path to the file.</param>
|
||||
/// <returns>A key combining the containing directory and file name without extension.</returns>
|
||||
private static string GetVobSubPairingKey(string file)
|
||||
{
|
||||
var directory = Path.GetDirectoryName(file) ?? string.Empty;
|
||||
var baseName = Path.GetFileNameWithoutExtension(file);
|
||||
return Path.Combine(directory, baseName);
|
||||
}
|
||||
|
||||
/// <summary>
|
||||
/// Returns the media info of the given file.
|
||||
/// </summary>
|
||||
|
||||
@@ -179,6 +179,146 @@ public class MediaInfoResolverTests
|
||||
Assert.Empty(streams);
|
||||
}
|
||||
|
||||
[Fact]
|
||||
public void GetExternalFiles_VobSubIdxAndSubPair_OnlyReturnsIdxFile()
|
||||
{
|
||||
// VobSub (.sub) payloads only carry per-track language metadata when read
|
||||
// alongside their paired .idx index file. When both are present, only the
|
||||
// .idx file should be returned so it (not the raw .sub) gets probed.
|
||||
BaseItem.MediaSourceManager = Mock.Of<IMediaSourceManager>();
|
||||
|
||||
var video = new Movie
|
||||
{
|
||||
Path = VideoDirectoryPath + "/My.Video.mkv"
|
||||
};
|
||||
|
||||
var directoryService = new Mock<IDirectoryService>(MockBehavior.Strict);
|
||||
directoryService.Setup(ds => ds.GetFilePaths(It.IsRegex(VideoDirectoryRegex), It.IsAny<bool>()))
|
||||
.Returns(new[] { VideoDirectoryPath + "/My.Video.idx", VideoDirectoryPath + "/My.Video.sub" });
|
||||
directoryService.Setup(ds => ds.GetFilePaths(It.IsRegex(MetadataDirectoryRegex), It.IsAny<bool>()))
|
||||
.Returns(Array.Empty<string>());
|
||||
|
||||
var streams = _subtitleResolver.GetExternalFiles(video, directoryService.Object, false).ToList();
|
||||
|
||||
var stream = Assert.Single(streams);
|
||||
Assert.EndsWith(".idx", stream.Path, StringComparison.OrdinalIgnoreCase);
|
||||
}
|
||||
|
||||
[Fact]
|
||||
public void GetExternalFiles_VobSubIdxWithoutMatchingSub_DoesNotReturnIdxFile()
|
||||
{
|
||||
// An .idx file with no paired .sub cannot be probed for anything, so it must be
|
||||
// left out entirely rather than surfaced as a doomed-to-fail probe candidate.
|
||||
// Surfacing it anyway would also make it "exist" from Jellyfin's perspective
|
||||
// even after the real .sub is deleted, preventing stale subtitle stream data
|
||||
// from ever being cleared on a rescan.
|
||||
BaseItem.MediaSourceManager = Mock.Of<IMediaSourceManager>();
|
||||
|
||||
var video = new Movie
|
||||
{
|
||||
Path = VideoDirectoryPath + "/My.Video.mkv"
|
||||
};
|
||||
|
||||
var directoryService = GetDirectoryServiceForExternalFile("My.Video.idx");
|
||||
var streams = _subtitleResolver.GetExternalFiles(video, directoryService, false).ToList();
|
||||
|
||||
Assert.Empty(streams);
|
||||
}
|
||||
|
||||
[Fact]
|
||||
public void GetExternalFiles_StandaloneSubWithoutIdx_StillReturnsSubFile()
|
||||
{
|
||||
// Guards against the .idx/.sub pairing suppression firing when there is no
|
||||
// .idx sidecar at all - a lone .sub file must still be returned.
|
||||
BaseItem.MediaSourceManager = Mock.Of<IMediaSourceManager>();
|
||||
|
||||
var video = new Movie
|
||||
{
|
||||
Path = VideoDirectoryPath + "/My.Video.mkv"
|
||||
};
|
||||
|
||||
var directoryService = GetDirectoryServiceForExternalFile("My.Video.sub");
|
||||
var streams = _subtitleResolver.GetExternalFiles(video, directoryService, false).ToList();
|
||||
|
||||
var stream = Assert.Single(streams);
|
||||
Assert.EndsWith(".sub", stream.Path, StringComparison.OrdinalIgnoreCase);
|
||||
}
|
||||
|
||||
[Fact]
|
||||
public void GetExternalFiles_VobSubIdxAndSubInDifferentDirectories_DoesNotPair()
|
||||
{
|
||||
// A same-named .idx and .sub split across the video folder and the internal
|
||||
// metadata folder cannot be paired by ffprobe (it only looks next to the .idx),
|
||||
// so the .sub must still be returned, but the orphaned .idx (no sibling .sub in
|
||||
// its own directory) must be left out since it cannot be probed.
|
||||
BaseItem.MediaSourceManager = Mock.Of<IMediaSourceManager>();
|
||||
|
||||
var video = new Movie
|
||||
{
|
||||
Path = VideoDirectoryPath + "/My.Video.mkv"
|
||||
};
|
||||
|
||||
var directoryService = new Mock<IDirectoryService>(MockBehavior.Strict);
|
||||
directoryService.Setup(ds => ds.GetFilePaths(It.IsRegex(VideoDirectoryRegex), It.IsAny<bool>()))
|
||||
.Returns(new[] { VideoDirectoryPath + "/My.Video.sub" });
|
||||
directoryService.Setup(ds => ds.GetFilePaths(It.IsRegex(MetadataDirectoryRegex), It.IsAny<bool>()))
|
||||
.Returns(new[] { MetadataDirectoryPath + "/My.Video.idx" });
|
||||
|
||||
var streams = _subtitleResolver.GetExternalFiles(video, directoryService.Object, false).ToList();
|
||||
|
||||
var stream = Assert.Single(streams);
|
||||
Assert.EndsWith(".sub", stream.Path, StringComparison.OrdinalIgnoreCase);
|
||||
}
|
||||
|
||||
[Fact]
|
||||
public void GetExternalFiles_VobSubIdxAndSubWithMatchingLanguageFlag_SuppressesSub()
|
||||
{
|
||||
// A .idx/.sub pair sharing the same filename flags (e.g. a language token) should
|
||||
// still pair and suppress the .sub, just like an unflagged pair.
|
||||
BaseItem.MediaSourceManager = Mock.Of<IMediaSourceManager>();
|
||||
|
||||
var video = new Movie
|
||||
{
|
||||
Path = VideoDirectoryPath + "/My.Video.mkv"
|
||||
};
|
||||
|
||||
var directoryService = new Mock<IDirectoryService>(MockBehavior.Strict);
|
||||
directoryService.Setup(ds => ds.GetFilePaths(It.IsRegex(VideoDirectoryRegex), It.IsAny<bool>()))
|
||||
.Returns(new[] { VideoDirectoryPath + "/My.Video.en.idx", VideoDirectoryPath + "/My.Video.en.sub" });
|
||||
directoryService.Setup(ds => ds.GetFilePaths(It.IsRegex(MetadataDirectoryRegex), It.IsAny<bool>()))
|
||||
.Returns(Array.Empty<string>());
|
||||
|
||||
var streams = _subtitleResolver.GetExternalFiles(video, directoryService.Object, false).ToList();
|
||||
|
||||
var stream = Assert.Single(streams);
|
||||
Assert.EndsWith(".idx", stream.Path, StringComparison.OrdinalIgnoreCase);
|
||||
}
|
||||
|
||||
[Fact]
|
||||
public void GetExternalFiles_VobSubIdxAndSubWithMismatchedNames_DoesNotPair()
|
||||
{
|
||||
// An .idx and .sub with different basenames (e.g. differing filename flags) are not
|
||||
// a pair ffprobe would resolve. The .sub must still be returned, but the orphaned
|
||||
// .idx (no same-named sibling .sub) must be left out since it cannot be probed.
|
||||
BaseItem.MediaSourceManager = Mock.Of<IMediaSourceManager>();
|
||||
|
||||
var video = new Movie
|
||||
{
|
||||
Path = VideoDirectoryPath + "/My.Video.mkv"
|
||||
};
|
||||
|
||||
var directoryService = new Mock<IDirectoryService>(MockBehavior.Strict);
|
||||
directoryService.Setup(ds => ds.GetFilePaths(It.IsRegex(VideoDirectoryRegex), It.IsAny<bool>()))
|
||||
.Returns(new[] { VideoDirectoryPath + "/My.Video.idx", VideoDirectoryPath + "/My.Video.en.sub" });
|
||||
directoryService.Setup(ds => ds.GetFilePaths(It.IsRegex(MetadataDirectoryRegex), It.IsAny<bool>()))
|
||||
.Returns(Array.Empty<string>());
|
||||
|
||||
var streams = _subtitleResolver.GetExternalFiles(video, directoryService.Object, false).ToList();
|
||||
|
||||
var stream = Assert.Single(streams);
|
||||
Assert.EndsWith(".sub", stream.Path, StringComparison.OrdinalIgnoreCase);
|
||||
}
|
||||
|
||||
[Theory]
|
||||
[InlineData("https://url.com/My.Video.mkv")]
|
||||
[InlineData(VideoDirectoryPath)] // valid but no files found for this test
|
||||
|
||||
Reference in New Issue
Block a user