From 006809f02a41064b8c7ca64ce7391c9296ac1074 Mon Sep 17 00:00:00 2001 From: Shadowghost Date: Sat, 5 Sep 2026 19:03:43 +0200 Subject: [PATCH 1/2] Fix handling of unordered multi-episode NFOs --- .../TV/EpisodeMetadataService.cs | 27 +++++++ .../Parsers/EpisodeNfoParser.cs | 69 +++++++++++------ .../TV/EpisodeMetadataServiceTests.cs | 74 ++++++++++++++++++- .../Parsers/EpisodeNfoProviderTests.cs | 21 ++++++ .../Test Data/Rising-Reversed.nfo | 43 +++++++++++ 5 files changed, 209 insertions(+), 25 deletions(-) create mode 100644 tests/Jellyfin.XbmcMetadata.Tests/Test Data/Rising-Reversed.nfo diff --git a/MediaBrowser.Providers/TV/EpisodeMetadataService.cs b/MediaBrowser.Providers/TV/EpisodeMetadataService.cs index 596ca8d201..4ec6afe45b 100644 --- a/MediaBrowser.Providers/TV/EpisodeMetadataService.cs +++ b/MediaBrowser.Providers/TV/EpisodeMetadataService.cs @@ -44,6 +44,33 @@ public class EpisodeMetadataService : MetadataService { var updatedType = base.BeforeSaveInternal(item, isFullRefresh, updateType); + // An episode cannot end before it starts. Providers and nfo files occasionally report the range + // transposed, which makes clients render the episode number backwards. Both numbers describe the + // same set of episodes either way, so restore their order instead of dropping the range. + if (item.IndexNumber.HasValue && item.IndexNumberEnd < item.IndexNumber) + { + Logger.LogWarning( + "Correcting reversed episode range {IndexNumber}-{IndexNumberEnd} for {Path}", + item.IndexNumber, + item.IndexNumberEnd, + item.Path); + + (item.IndexNumber, item.IndexNumberEnd) = (item.IndexNumberEnd, item.IndexNumber); + updatedType |= ItemUpdateType.MetadataImport; + } + else if (item.IndexNumberEnd.HasValue && !item.IndexNumber.HasValue) + { + // Without a first episode the end does not describe a range. Promoting it to the episode number + // would invent an identity the metadata never supplied, so drop the orphaned value instead. + Logger.LogWarning( + "Discarding episode range end {IndexNumberEnd} without an episode number for {Path}", + item.IndexNumberEnd, + item.Path); + + item.IndexNumberEnd = null; + updatedType |= ItemUpdateType.MetadataImport; + } + var seriesName = item.FindSeriesName(); if (!string.Equals(item.SeriesName, seriesName, StringComparison.Ordinal)) { diff --git a/MediaBrowser.XbmcMetadata/Parsers/EpisodeNfoParser.cs b/MediaBrowser.XbmcMetadata/Parsers/EpisodeNfoParser.cs index 19b1bbe7b6..c3e5c791c0 100644 --- a/MediaBrowser.XbmcMetadata/Parsers/EpisodeNfoParser.cs +++ b/MediaBrowser.XbmcMetadata/Parsers/EpisodeNfoParser.cs @@ -1,5 +1,7 @@ using System; +using System.Collections.Generic; using System.IO; +using System.Linq; using System.Text; using System.Threading; using System.Xml; @@ -44,41 +46,60 @@ namespace MediaBrowser.XbmcMetadata.Parsers var xmlFile = File.ReadAllText(metadataFile); - var srch = ""; - var index = xmlFile.IndexOf(srch, StringComparison.OrdinalIgnoreCase); - - var xml = xmlFile; - - if (index != -1) + // Split the nfo into its episodedetails blocks. + // This is needed because XBMC metadata uses multiple episodedetails blocks instead of an episodenumberend tag. + const string Srch = ""; + var blocks = new List(); + int index; + while ((index = xmlFile.IndexOf(Srch, StringComparison.OrdinalIgnoreCase)) != -1) { - xml = xmlFile.Substring(0, index + srch.Length); - xmlFile = xmlFile.Substring(index + srch.Length); + blocks.Add(xmlFile.Substring(0, index + Srch.Length)); + xmlFile = xmlFile.Substring(index + Srch.Length); + } + + if (blocks.Count == 0) + { + // No closing tag, let the xml reader deal with whatever is in the file + blocks.Add(xmlFile); } // These are not going to be valid xml so no sense in causing the provider to fail and spamming the log with exceptions try { - // Extract episode details from the first episodedetails block - ReadEpisodeDetailsFromXml(item, xml, settings, cancellationToken); + if (blocks.Count == 1) + { + ReadEpisodeDetailsFromXml(item, blocks[0], settings, cancellationToken); + return; + } - // Extract the last episode number from nfo - // Retrieves all additional episodedetails blocks from the rest of the nfo and concatenates the name, originalTitle and overview tags with the first episode - // This is needed because XBMC metadata uses multiple episodedetails blocks instead of episodenumberend tag + // The blocks are not guaranteed to be written in ascending episode order, so parse them all + // and sort them before merging. + var episodes = blocks + .Select(block => + { + var episode = new MetadataResult() + { + Item = new Episode() + }; + + ReadEpisodeDetailsFromXml(episode, block, settings, cancellationToken); + + return (Xml: block, Result: episode); + }) + .OrderBy(episode => episode.Result.Item.IndexNumber ?? int.MaxValue) + .ToList(); + + // Extract the details of the lowest numbered episode into the item that is returned to the caller + ReadEpisodeDetailsFromXml(item, episodes[0].Xml, settings, cancellationToken); + + // Concatenate the name, originalTitle and overview tags of the remaining episodes with the first one + // and take the highest episode number as the last episode of the file var name = new StringBuilder(item.Item.Name); var originalTitle = new StringBuilder(item.Item.OriginalTitle); var overview = new StringBuilder(item.Item.Overview); - while ((index = xmlFile.IndexOf(srch, StringComparison.OrdinalIgnoreCase)) != -1) + for (var i = 1; i < episodes.Count; i++) { - xml = xmlFile.Substring(0, index + srch.Length); - xmlFile = xmlFile.Substring(index + srch.Length); - - var additionalEpisode = new MetadataResult() - { - Item = new Episode() - }; - - // Extract episode details from additional episodedetails block - ReadEpisodeDetailsFromXml(additionalEpisode, xml, settings, cancellationToken); + var additionalEpisode = episodes[i].Result; if (!string.IsNullOrEmpty(additionalEpisode.Item.Name)) { diff --git a/tests/Jellyfin.Providers.Tests/TV/EpisodeMetadataServiceTests.cs b/tests/Jellyfin.Providers.Tests/TV/EpisodeMetadataServiceTests.cs index 8f5b1b3c48..24fd6a58e9 100644 --- a/tests/Jellyfin.Providers.Tests/TV/EpisodeMetadataServiceTests.cs +++ b/tests/Jellyfin.Providers.Tests/TV/EpisodeMetadataServiceTests.cs @@ -1,5 +1,6 @@ using System; using MediaBrowser.Controller.Configuration; +using MediaBrowser.Controller.Entities; using MediaBrowser.Controller.Entities.TV; using MediaBrowser.Controller.IO; using MediaBrowser.Controller.Library; @@ -15,9 +16,23 @@ using Xunit; namespace Jellyfin.Providers.Tests.TV; -public class EpisodeMetadataServiceTests +// put tests that mock the static LibraryManager in the same collection to avoid test interference +[Collection("LibraryManagerTests")] +public sealed class EpisodeMetadataServiceTests : IDisposable { private readonly TestEpisodeMetadataService _service = new(); + private readonly ILibraryManager? _previousLibraryManager; + + public EpisodeMetadataServiceTests() + { + _previousLibraryManager = BaseItem.LibraryManager; + BaseItem.LibraryManager = Mock.Of(); + } + + public void Dispose() + { + BaseItem.LibraryManager = _previousLibraryManager; + } [Fact] public void MergeData_ProviderSeasonOverridesPathDerivedSeason() @@ -88,6 +103,58 @@ public class EpisodeMetadataServiceTests Assert.Equal(1, target.Item.ParentIndexNumber); } + [Theory] + [InlineData(2, 1)] // e.g. an nfo with its episodedetails blocks in descending order + [InlineData(22, 21)] + public void BeforeSave_ReversedEpisodeRange_RestoresOrder(int indexNumber, int indexNumberEnd) + { + var item = new Episode + { + IndexNumber = indexNumber, + IndexNumberEnd = indexNumberEnd + }; + + var updateType = _service.BeforeSave(item); + + // The range still covers the same episodes, it is just no longer transposed + Assert.Equal(indexNumberEnd, item.IndexNumber); + Assert.Equal(indexNumber, item.IndexNumberEnd); + Assert.True(updateType.HasFlag(ItemUpdateType.MetadataImport)); + } + + [Fact] + public void BeforeSave_EpisodeRangeWithoutStart_ClearsIndexNumberEnd() + { + var item = new Episode + { + IndexNumber = null, + IndexNumberEnd = 2 + }; + + var updateType = _service.BeforeSave(item); + + Assert.Null(item.IndexNumberEnd); + Assert.Null(item.IndexNumber); + Assert.True(updateType.HasFlag(ItemUpdateType.MetadataImport)); + } + + [Theory] + [InlineData(1, 2)] // Regular multi episode file + [InlineData(1, 1)] // Degenerate but not contradictory + public void BeforeSave_ValidEpisodeRange_KeepsIndexNumberEnd(int indexNumber, int indexNumberEnd) + { + var item = new Episode + { + IndexNumber = indexNumber, + IndexNumberEnd = indexNumberEnd + }; + + _service.BeforeSave(item); + + Assert.Equal(indexNumber, item.IndexNumber); + Assert.Equal(indexNumberEnd, item.IndexNumberEnd); + } + private sealed class TestEpisodeMetadataService : EpisodeMetadataService { public TestEpisodeMetadataService() @@ -106,5 +173,10 @@ public class EpisodeMetadataServiceTests { MergeData(source, target, Array.Empty(), replaceData, mergeMetadataSettings); } + + public ItemUpdateType BeforeSave(Episode item) + { + return BeforeSaveInternal(item, false, ItemUpdateType.None); + } } } diff --git a/tests/Jellyfin.XbmcMetadata.Tests/Parsers/EpisodeNfoProviderTests.cs b/tests/Jellyfin.XbmcMetadata.Tests/Parsers/EpisodeNfoProviderTests.cs index a04b37f215..3767b5c954 100644 --- a/tests/Jellyfin.XbmcMetadata.Tests/Parsers/EpisodeNfoProviderTests.cs +++ b/tests/Jellyfin.XbmcMetadata.Tests/Parsers/EpisodeNfoProviderTests.cs @@ -123,6 +123,27 @@ namespace Jellyfin.XbmcMetadata.Tests.Parsers Assert.Equal(2004, item.ProductionYear); } + [Fact] + public void Fetch_Valid_MultiEpisode_Unordered_Success() + { + var result = new MetadataResult() + { + Item = new Episode() + }; + + _parser.Fetch(result, "Test Data/Rising-Reversed.nfo", CancellationToken.None); + + var item = result.Item; + // The episodedetails blocks are stored in descending order, the merged episode must still be in ascending order + Assert.Equal("Rising (1) / Rising (2)", item.Name); + Assert.Equal(1, item.IndexNumber); + Assert.Equal(2, item.IndexNumberEnd); + Assert.Equal(1, item.ParentIndexNumber); + Assert.Equal("A new Stargate team embarks on a dangerous mission to a distant galaxy, where they discover a mythical lost city -- and a deadly new enemy. / Sheppard tries to convince Weir to mount a rescue mission to free Colonel Sumner, Teyla, and the others captured by the Wraith.", item.Overview); + Assert.Equal(new DateTime(2004, 7, 16), item.PremiereDate); + Assert.Equal(2004, item.ProductionYear); + } + [Fact] public void Fetch_Valid_MultiEpisode_With_Missing_Tags_Success() { diff --git a/tests/Jellyfin.XbmcMetadata.Tests/Test Data/Rising-Reversed.nfo b/tests/Jellyfin.XbmcMetadata.Tests/Test Data/Rising-Reversed.nfo new file mode 100644 index 0000000000..6dbab13566 --- /dev/null +++ b/tests/Jellyfin.XbmcMetadata.Tests/Test Data/Rising-Reversed.nfo @@ -0,0 +1,43 @@ + + Rising (2) + 1 + 2 + 2004-07-16 + Sheppard tries to convince Weir to mount a rescue mission to free Colonel Sumner, Teyla, and the others captured by the Wraith. + https://artworks.thetvdb.com/banners/episodes/70851/25334.jpg + false + 7.9 + + Joe Flanigan + John Sheppard + 0 + https://image.tmdb.org/t/p/w300_and_h450_bestv2/5AA1ORKIsnMakT6fCVy3JKlzMs6.jpg + + + David Hewlett + Rodney McKay + 1 + https://image.tmdb.org/t/p/w300_and_h450_bestv2/hUcYyssAPCqnZ4GjolhOWXHTWSa.jpg + + + Rising (1) + 1 + 1 + 2004-07-16 + A new Stargate team embarks on a dangerous mission to a distant galaxy, where they discover a mythical lost city -- and a deadly new enemy. + https://artworks.thetvdb.com/banners/episodes/70851/25333.jpg + false + 8.0 + + Joe Flanigan + John Sheppard + 0 + https://image.tmdb.org/t/p/w300_and_h450_bestv2/5AA1ORKIsnMakT6fCVy3JKlzMs6.jpg + + + David Hewlett + Rodney McKay + 1 + https://image.tmdb.org/t/p/w300_and_h450_bestv2/hUcYyssAPCqnZ4GjolhOWXHTWSa.jpg + + From 79a6ac6d82499814ca2547bdd3872d24c2f9b08d Mon Sep 17 00:00:00 2001 From: Shadowghost Date: Sun, 6 Sep 2026 08:04:34 +0200 Subject: [PATCH 2/2] Don't read episode markers in episode titles as a multi-episode range --- Emby.Naming/TV/EpisodePathParser.cs | 6 ++++-- MediaBrowser.Providers/TV/EpisodeMetadataService.cs | 12 +++++------- tests/Jellyfin.Naming.Tests/TV/MultiEpisodeTests.cs | 3 +++ .../TV/EpisodeMetadataServiceTests.cs | 11 ++++++----- 4 files changed, 18 insertions(+), 14 deletions(-) diff --git a/Emby.Naming/TV/EpisodePathParser.cs b/Emby.Naming/TV/EpisodePathParser.cs index 0c737964b4..f06aa909ba 100644 --- a/Emby.Naming/TV/EpisodePathParser.cs +++ b/Emby.Naming/TV/EpisodePathParser.cs @@ -158,7 +158,9 @@ namespace Emby.Naming.TV if (nextIndex >= name.Length || !"0123456789iIpP".Contains(name[nextIndex], StringComparison.Ordinal)) { - if (int.TryParse(endingNumberGroup.ValueSpan, NumberStyles.Integer, CultureInfo.InvariantCulture, out num)) + // A range cannot end before it starts, so a lower number belongs to the episode title rather than to a range. + if (int.TryParse(endingNumberGroup.ValueSpan, NumberStyles.Integer, CultureInfo.InvariantCulture, out num) + && num >= result.EpisodeNumber) { result.EndingEpisodeNumber = num; } @@ -226,7 +228,7 @@ namespace Emby.Naming.TV info.SeriesName = result.SeriesName; } - if (!info.EndingEpisodeNumber.HasValue && info.EpisodeNumber.HasValue) + if (!info.EndingEpisodeNumber.HasValue && result.EndingEpisodeNumber >= info.EpisodeNumber) { info.EndingEpisodeNumber = result.EndingEpisodeNumber; } diff --git a/MediaBrowser.Providers/TV/EpisodeMetadataService.cs b/MediaBrowser.Providers/TV/EpisodeMetadataService.cs index 4ec6afe45b..f662ac2367 100644 --- a/MediaBrowser.Providers/TV/EpisodeMetadataService.cs +++ b/MediaBrowser.Providers/TV/EpisodeMetadataService.cs @@ -44,18 +44,16 @@ public class EpisodeMetadataService : MetadataService { var updatedType = base.BeforeSaveInternal(item, isFullRefresh, updateType); - // An episode cannot end before it starts. Providers and nfo files occasionally report the range - // transposed, which makes clients render the episode number backwards. Both numbers describe the - // same set of episodes either way, so restore their order instead of dropping the range. - if (item.IndexNumber.HasValue && item.IndexNumberEnd < item.IndexNumber) + // An episode cannot end before it starts. + if (item.IndexNumberEnd < item.IndexNumber) { Logger.LogWarning( - "Correcting reversed episode range {IndexNumber}-{IndexNumberEnd} for {Path}", - item.IndexNumber, + "Discarding episode range end {IndexNumberEnd} preceding episode number {IndexNumber} for {Path}", item.IndexNumberEnd, + item.IndexNumber, item.Path); - (item.IndexNumber, item.IndexNumberEnd) = (item.IndexNumberEnd, item.IndexNumber); + item.IndexNumberEnd = null; updatedType |= ItemUpdateType.MetadataImport; } else if (item.IndexNumberEnd.HasValue && !item.IndexNumber.HasValue) diff --git a/tests/Jellyfin.Naming.Tests/TV/MultiEpisodeTests.cs b/tests/Jellyfin.Naming.Tests/TV/MultiEpisodeTests.cs index 7e708c681d..4236749423 100644 --- a/tests/Jellyfin.Naming.Tests/TV/MultiEpisodeTests.cs +++ b/tests/Jellyfin.Naming.Tests/TV/MultiEpisodeTests.cs @@ -74,6 +74,9 @@ namespace Jellyfin.Naming.Tests.TV [InlineData("Season 5/S05E23 11-59 [HDTV-1080p][x265 AC3].mkv", null)] [InlineData("Season 5/S05E23 11-59 [HDTV-1080p][HEVC AC3].mkv", null)] [InlineData("Season 1/S01E01 1-23-45 [Bluray-1080p][AV1 Opus].mkv", null)] + // Episode markers in the episode title must not be read as an episode range + [InlineData("Season 03/Star Trek Enterprise (2001) - S03E21 - E2 (1080p BluRay x265).mkv", null)] + [InlineData("Season 02/Series Name (2001) - S02E10 - E5 [WEBRip-1080p].mkv", null)] public void TestGetEndingEpisodeNumberFromFile(string filename, int? endingEpisodeNumber) { var result = _episodePathParser.Parse(filename, false); diff --git a/tests/Jellyfin.Providers.Tests/TV/EpisodeMetadataServiceTests.cs b/tests/Jellyfin.Providers.Tests/TV/EpisodeMetadataServiceTests.cs index 24fd6a58e9..ea762256db 100644 --- a/tests/Jellyfin.Providers.Tests/TV/EpisodeMetadataServiceTests.cs +++ b/tests/Jellyfin.Providers.Tests/TV/EpisodeMetadataServiceTests.cs @@ -104,9 +104,10 @@ public sealed class EpisodeMetadataServiceTests : IDisposable } [Theory] - [InlineData(2, 1)] // e.g. an nfo with its episodedetails blocks in descending order + [InlineData(2, 1)] [InlineData(22, 21)] - public void BeforeSave_ReversedEpisodeRange_RestoresOrder(int indexNumber, int indexNumberEnd) + [InlineData(21, 2)] // e.g. "Series - S03E21 - E2 (1080p BluRay x265).mkv", where "E2" is the episode title + public void BeforeSave_ReversedEpisodeRange_ClearsIndexNumberEnd(int indexNumber, int indexNumberEnd) { var item = new Episode { @@ -116,9 +117,9 @@ public sealed class EpisodeMetadataServiceTests : IDisposable var updateType = _service.BeforeSave(item); - // The range still covers the same episodes, it is just no longer transposed - Assert.Equal(indexNumberEnd, item.IndexNumber); - Assert.Equal(indexNumber, item.IndexNumberEnd); + // The episode number identifies the item, so it is kept and the impossible range is dropped + Assert.Equal(indexNumber, item.IndexNumber); + Assert.Null(item.IndexNumberEnd); Assert.True(updateType.HasFlag(ItemUpdateType.MetadataImport)); }