From 0b20d7a05bb174181ef56e9c2ee08a590a74528e Mon Sep 17 00:00:00 2001 From: Oskar Bali <51484222+Oggeb1@users.noreply.github.com> Date: Wed, 19 Aug 2026 11:29:52 +0200 Subject: [PATCH 01/17] Fix BDMV PGS subtitles with TrueHD Add myself CONTRIBUTORS.md --- CONTRIBUTORS.md | 1 + .../MediaEncoding/EncodingHelper.cs | 19 ++++++++++++++++++- .../Subtitles/SubtitleEncoder.cs | 4 ++-- 3 files changed, 21 insertions(+), 3 deletions(-) diff --git a/CONTRIBUTORS.md b/CONTRIBUTORS.md index 99f24e3a14..a97d335170 100644 --- a/CONTRIBUTORS.md +++ b/CONTRIBUTORS.md @@ -240,6 +240,7 @@ - [Florin-Popescu](https://github.com/Florin-Popescu) - [m0g3r](https://github.com/m0g3r) - [martin-77](https://github.com/martin-77) + - [Oggeb1](https://github.com/Oggeb1) # Emby Contributors diff --git a/MediaBrowser.Controller/MediaEncoding/EncodingHelper.cs b/MediaBrowser.Controller/MediaEncoding/EncodingHelper.cs index 9a68889352..9e6d4a2899 100644 --- a/MediaBrowser.Controller/MediaEncoding/EncodingHelper.cs +++ b/MediaBrowser.Controller/MediaEncoding/EncodingHelper.cs @@ -6311,7 +6311,7 @@ namespace MediaBrowser.Controller.MediaEncoding string.Join(',', overlayFilters)); var mapPrefix = Convert.ToInt32(state.SubtitleStream.IsExternal); - var subtitleStreamIndex = FindIndex(state.MediaSource.MediaStreams, state.SubtitleStream); + var subtitleStreamIndex = GetSubtitleStreamIndexForFfmpeg(state.MediaSource, state.SubtitleStream); var videoStreamIndex = FindIndex(state.MediaSource.MediaStreams, state.VideoStream); if (hasSubs) @@ -7943,6 +7943,23 @@ namespace MediaBrowser.Controller.MediaEncoding return -1; } + public static int GetSubtitleStreamIndexForFfmpeg(MediaSourceInfo mediaSource, MediaStream subtitleStream) + { + var index = FindIndex(mediaSource.MediaStreams, subtitleStream); + if (index == -1 || mediaSource.VideoType != VideoType.BluRay) + { + return index; + } + + var hiddenStreamsBefore = mediaSource.MediaStreams.Count(s => + s.Type == MediaStreamType.Audio + && (string.Equals(s.Codec, "truehd", StringComparison.OrdinalIgnoreCase) + || string.Equals(s.Codec, "atmos", StringComparison.OrdinalIgnoreCase)) + && s.Index < subtitleStream.Index); + + return index + hiddenStreamsBefore; + } + public static bool IsCopyCodec(string codec) { return string.Equals(codec, "copy", StringComparison.OrdinalIgnoreCase); diff --git a/MediaBrowser.MediaEncoding/Subtitles/SubtitleEncoder.cs b/MediaBrowser.MediaEncoding/Subtitles/SubtitleEncoder.cs index bd516f0a9f..e733cfd8ac 100644 --- a/MediaBrowser.MediaEncoding/Subtitles/SubtitleEncoder.cs +++ b/MediaBrowser.MediaEncoding/Subtitles/SubtitleEncoder.cs @@ -648,7 +648,7 @@ namespace MediaBrowser.MediaEncoding.Subtitles List subtitleStreams, CancellationToken cancellationToken) { - var inputPath = _mediaEncoder.GetInputArgument(mediaSource.Path, mediaSource); + var inputPath = _mediaEncoder.GetInputPathArgument(mediaSource.Path, mediaSource); var outputPaths = new List(); var args = string.Format( CultureInfo.InvariantCulture, @@ -672,7 +672,7 @@ namespace MediaBrowser.MediaEncoding.Subtitles var outputCodec = IsCodecCopyable(subtitleStream.Codec) ? "copy" : "srt"; // FFmpeg does not provide an .idx/.sub muxer, so VobSub streams must be written as MKS files. var outputFormatOption = MediaStream.IsVobSubFormat(subtitleStream.Codec) ? " -f matroska" : string.Empty; - var streamIndex = EncodingHelper.FindIndex(mediaSource.MediaStreams, subtitleStream); + var streamIndex = EncodingHelper.GetSubtitleStreamIndexForFfmpeg(mediaSource, subtitleStream); if (streamIndex == -1) { From 163895b99f780df1ce8f50f3d0e17f4a2880e251 Mon Sep 17 00:00:00 2001 From: Oskar Bali <51484222+Oggeb1@users.noreply.github.com> Date: Sat, 29 Aug 2026 13:52:20 +0200 Subject: [PATCH 02/17] Fix BDMV with external subtitle GetSubtitleStreamIndexForFfmpeg treated external and internal subtitles the same. This made subtitles out of sync with the video. --- MediaBrowser.Controller/MediaEncoding/EncodingHelper.cs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/MediaBrowser.Controller/MediaEncoding/EncodingHelper.cs b/MediaBrowser.Controller/MediaEncoding/EncodingHelper.cs index 9e6d4a2899..cb01c1486b 100644 --- a/MediaBrowser.Controller/MediaEncoding/EncodingHelper.cs +++ b/MediaBrowser.Controller/MediaEncoding/EncodingHelper.cs @@ -7946,7 +7946,7 @@ namespace MediaBrowser.Controller.MediaEncoding public static int GetSubtitleStreamIndexForFfmpeg(MediaSourceInfo mediaSource, MediaStream subtitleStream) { var index = FindIndex(mediaSource.MediaStreams, subtitleStream); - if (index == -1 || mediaSource.VideoType != VideoType.BluRay) + if (index == -1 || subtitleStream.IsExternal || mediaSource.VideoType != VideoType.BluRay) { return index; } From 39c2885fd6d0ae4075ee24c00e6128fa0de9d92b Mon Sep 17 00:00:00 2001 From: Kyle Farnung Date: Sun, 30 Aug 2026 12:01:05 -0700 Subject: [PATCH 03/17] Probe .idx instead of .sub for external VobSub subtitle language detection External VobSub subtitle pairs (.idx and .sub) were only probed via the bare .sub file. In cases where multiple languages are present, this results in missing language metadata. Fix by detecting the matching .idx file during media info resolution to run ffprobe on that file and skip processing the .sub entirely. ffprobe will automatically find the matching (same directory, case-sensitive base) .sub file and process both. Added regression tests covering idx/sub pairing, unpaired files, cross-directory pairs, and language-flagged filenames. Fixes #17745 --- .../ExternalFiles/ExternalPathParser.cs | 9 +- .../MediaInfo/MediaInfoResolver.cs | 89 +++++++++++ .../MediaInfo/MediaInfoResolverTests.cs | 140 ++++++++++++++++++ 3 files changed, 237 insertions(+), 1 deletion(-) diff --git a/Emby.Naming/ExternalFiles/ExternalPathParser.cs b/Emby.Naming/ExternalFiles/ExternalPathParser.cs index 8e7da5db42..1f16161282 100644 --- a/Emby.Naming/ExternalFiles/ExternalPathParser.cs +++ b/Emby.Naming/ExternalFiles/ExternalPathParser.cs @@ -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))) { diff --git a/MediaBrowser.Providers/MediaInfo/MediaInfoResolver.cs b/MediaBrowser.Providers/MediaInfo/MediaInfoResolver.cs index 6f9d5f19da..ecb6e5d990 100644 --- a/MediaBrowser.Providers/MediaInfo/MediaInfoResolver.cs +++ b/MediaBrowser.Providers/MediaInfo/MediaInfoResolver.cs @@ -231,10 +231,28 @@ namespace MediaBrowser.Providers.MediaInfo return Array.Empty(); } + // 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? pairedVobSubKeys = _type == DlnaProfileType.Subtitle + ? GetPairedVobSubKeys(files) + : null; + var externalPathInfos = new List(); ReadOnlySpan 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; } + /// + /// 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. + /// + /// The full path to the candidate file. + /// The set of pairing keys with both an .idx and .sub present, or null if not applicable. + /// true if the file should be suppressed; otherwise, false. + private static bool IsSuppressedVobSubFile(string file, HashSet? 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; + } + + /// + /// 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. + /// + /// The candidate files to search. + /// The set of pairing keys with both an .idx and .sub present. + private static HashSet GetPairedVobSubKeys(IEnumerable files) + { + var idxKeys = new HashSet(StringComparer.Ordinal); + var subKeys = new HashSet(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; + } + + /// + /// Builds a directory+basename key used to pair a VobSub .idx file with its .sub + /// payload only when both live in the same directory. + /// + /// The full path to the file. + /// A key combining the containing directory and file name without extension. + private static string GetVobSubPairingKey(string file) + { + var directory = Path.GetDirectoryName(file) ?? string.Empty; + var baseName = Path.GetFileNameWithoutExtension(file); + return Path.Combine(directory, baseName); + } + /// /// Returns the media info of the given file. /// diff --git a/tests/Jellyfin.Providers.Tests/MediaInfo/MediaInfoResolverTests.cs b/tests/Jellyfin.Providers.Tests/MediaInfo/MediaInfoResolverTests.cs index 876f18741f..ce451861ef 100644 --- a/tests/Jellyfin.Providers.Tests/MediaInfo/MediaInfoResolverTests.cs +++ b/tests/Jellyfin.Providers.Tests/MediaInfo/MediaInfoResolverTests.cs @@ -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(); + + var video = new Movie + { + Path = VideoDirectoryPath + "/My.Video.mkv" + }; + + var directoryService = new Mock(MockBehavior.Strict); + directoryService.Setup(ds => ds.GetFilePaths(It.IsRegex(VideoDirectoryRegex), It.IsAny())) + .Returns(new[] { VideoDirectoryPath + "/My.Video.idx", VideoDirectoryPath + "/My.Video.sub" }); + directoryService.Setup(ds => ds.GetFilePaths(It.IsRegex(MetadataDirectoryRegex), It.IsAny())) + .Returns(Array.Empty()); + + 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(); + + 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(); + + 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(); + + var video = new Movie + { + Path = VideoDirectoryPath + "/My.Video.mkv" + }; + + var directoryService = new Mock(MockBehavior.Strict); + directoryService.Setup(ds => ds.GetFilePaths(It.IsRegex(VideoDirectoryRegex), It.IsAny())) + .Returns(new[] { VideoDirectoryPath + "/My.Video.sub" }); + directoryService.Setup(ds => ds.GetFilePaths(It.IsRegex(MetadataDirectoryRegex), It.IsAny())) + .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(); + + var video = new Movie + { + Path = VideoDirectoryPath + "/My.Video.mkv" + }; + + var directoryService = new Mock(MockBehavior.Strict); + directoryService.Setup(ds => ds.GetFilePaths(It.IsRegex(VideoDirectoryRegex), It.IsAny())) + .Returns(new[] { VideoDirectoryPath + "/My.Video.en.idx", VideoDirectoryPath + "/My.Video.en.sub" }); + directoryService.Setup(ds => ds.GetFilePaths(It.IsRegex(MetadataDirectoryRegex), It.IsAny())) + .Returns(Array.Empty()); + + 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(); + + var video = new Movie + { + Path = VideoDirectoryPath + "/My.Video.mkv" + }; + + var directoryService = new Mock(MockBehavior.Strict); + directoryService.Setup(ds => ds.GetFilePaths(It.IsRegex(VideoDirectoryRegex), It.IsAny())) + .Returns(new[] { VideoDirectoryPath + "/My.Video.idx", VideoDirectoryPath + "/My.Video.en.sub" }); + directoryService.Setup(ds => ds.GetFilePaths(It.IsRegex(MetadataDirectoryRegex), It.IsAny())) + .Returns(Array.Empty()); + + 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 From c5f8a93513c48501c24b192a987c7467d8a98608 Mon Sep 17 00:00:00 2001 From: Shadowghost Date: Tue, 1 Sep 2026 20:47:35 +0200 Subject: [PATCH 04/17] Close a change batch on its own window so a library scan cannot grow it without bound --- .../EntryPoints/LibraryChangedNotifier.cs | 65 +++++---- .../EntryPoints/UserDataChangeNotifier.cs | 66 ++++++---- .../LibraryChangedNotifierTests.cs | 123 ++++++++++++++++++ .../UserDataChangeNotifierTests.cs | 78 +++++++++++ 4 files changed, 283 insertions(+), 49 deletions(-) create mode 100644 tests/Jellyfin.Server.Implementations.Tests/EntryPoints/LibraryChangedNotifierTests.cs create mode 100644 tests/Jellyfin.Server.Implementations.Tests/EntryPoints/UserDataChangeNotifierTests.cs diff --git a/Emby.Server.Implementations/EntryPoints/LibraryChangedNotifier.cs b/Emby.Server.Implementations/EntryPoints/LibraryChangedNotifier.cs index 933cfc8cbe..02b104756e 100644 --- a/Emby.Server.Implementations/EntryPoints/LibraryChangedNotifier.cs +++ b/Emby.Server.Implementations/EntryPoints/LibraryChangedNotifier.cs @@ -27,6 +27,11 @@ namespace Emby.Server.Implementations.EntryPoints; /// public sealed class LibraryChangedNotifier : IHostedService, IDisposable { + // A batch holds a live reference to every item it names, so it has to stay small enough that a + // library scan - which changes items faster than any batch window closes - cannot grow it without + // bound. Reached only by a scan; interactive use closes a batch on the window long before this. + internal const int MaxBatchSize = 2000; + private readonly ILibraryManager _libraryManager; private readonly IServerConfigurationManager _configurationManager; private readonly IProviderManager _providerManager; @@ -35,11 +40,11 @@ public sealed class LibraryChangedNotifier : IHostedService, IDisposable private readonly ILogger _logger; private readonly Lock _libraryChangedSyncLock = new(); - private readonly List _foldersAddedTo = new(); - private readonly List _foldersRemovedFrom = new(); - private readonly List _itemsAdded = new(); - private readonly List _itemsRemoved = new(); - private readonly List _itemsUpdated = new(); + private readonly Dictionary _foldersAddedTo = []; + private readonly Dictionary _foldersRemovedFrom = []; + private readonly Dictionary _itemsAdded = []; + private readonly Dictionary _itemsRemoved = []; + private readonly Dictionary _itemsUpdated = []; private readonly ConcurrentDictionary _lastProgressMessageTimes = new(); private Timer? _libraryUpdateTimer; @@ -173,7 +178,7 @@ public sealed class LibraryChangedNotifier : IHostedService, IDisposable private void OnLibraryItemRemoved(object? sender, ItemChangeEventArgs e) => OnLibraryChange(e.Item, e.Parent, _itemsRemoved, _foldersRemovedFrom); - private void OnLibraryChange(BaseItem item, BaseItem parent, List itemsList, List? foldersList) + private void OnLibraryChange(BaseItem item, BaseItem parent, Dictionary itemsList, Dictionary? foldersList) { if (!FilterItem(item)) { @@ -182,23 +187,28 @@ public sealed class LibraryChangedNotifier : IHostedService, IDisposable lock (_libraryChangedSyncLock) { - var updateDuration = TimeSpan.FromSeconds(_configurationManager.Configuration.LibraryUpdateDuration); - + // The window runs from the first change of a batch and is never extended. Extending it on + // every change would keep a library scan's batch open for the whole scan, and the batch + // holds the items it names alive, so it would grow to the size of the library. if (_libraryUpdateTimer is null) { + var updateDuration = TimeSpan.FromSeconds(_configurationManager.Configuration.LibraryUpdateDuration); _libraryUpdateTimer = new Timer(LibraryUpdateTimerCallback, null, updateDuration, Timeout.InfiniteTimeSpan); } - else - { - _libraryUpdateTimer.Change(updateDuration, Timeout.InfiniteTimeSpan); - } if (foldersList is not null && parent is Folder folder) { - foldersList.Add(folder); + foldersList[folder.Id] = folder; } - itemsList.Add(item); + itemsList[item.Id] = item; + + // A window long enough to cover a burst still has to give way once the batch is large + // enough to be worth sending on its own. + if (_itemsAdded.Count + _itemsRemoved.Count + _itemsUpdated.Count >= MaxBatchSize) + { + _libraryUpdateTimer.Change(TimeSpan.Zero, Timeout.InfiniteTimeSpan); + } } } @@ -211,22 +221,16 @@ public sealed class LibraryChangedNotifier : IHostedService, IDisposable List itemsRemoved; lock (_libraryChangedSyncLock) { - // Remove dupes in case some were saved multiple times - foldersAddedTo = _foldersAddedTo - .DistinctBy(x => x.Id) - .ToList(); - - foldersRemovedFrom = _foldersRemovedFrom - .DistinctBy(x => x.Id) - .ToList(); + foldersAddedTo = _foldersAddedTo.Values.ToList(); + foldersRemovedFrom = _foldersRemovedFrom.Values.ToList(); itemsUpdated = _itemsUpdated - .Where(i => !_itemsAdded.Contains(i)) - .DistinctBy(x => x.Id) + .Where(e => !_itemsAdded.ContainsKey(e.Key)) + .Select(e => e.Value) .ToList(); - itemsAdded = _itemsAdded.ToList(); - itemsRemoved = _itemsRemoved.ToList(); + itemsAdded = _itemsAdded.Values.ToList(); + itemsRemoved = _itemsRemoved.Values.ToList(); if (_libraryUpdateTimer is not null) { @@ -241,6 +245,15 @@ public sealed class LibraryChangedNotifier : IHostedService, IDisposable _foldersRemovedFrom.Clear(); } + if (itemsAdded.Count == 0 + && itemsUpdated.Count == 0 + && itemsRemoved.Count == 0 + && foldersAddedTo.Count == 0 + && foldersRemovedFrom.Count == 0) + { + return; + } + await SendChangeNotifications(itemsAdded, itemsUpdated, itemsRemoved, foldersAddedTo, foldersRemovedFrom, CancellationToken.None).ConfigureAwait(false); } diff --git a/Emby.Server.Implementations/EntryPoints/UserDataChangeNotifier.cs b/Emby.Server.Implementations/EntryPoints/UserDataChangeNotifier.cs index fc174b7c14..b182e5837b 100644 --- a/Emby.Server.Implementations/EntryPoints/UserDataChangeNotifier.cs +++ b/Emby.Server.Implementations/EntryPoints/UserDataChangeNotifier.cs @@ -18,15 +18,17 @@ namespace Emby.Server.Implementations.EntryPoints public sealed class UserDataChangeNotifier : IHostedService, IDisposable { private const int UpdateDuration = 500; + internal const int MaxBatchSize = 2000; private readonly ISessionManager _sessionManager; private readonly IUserDataManager _userDataManager; private readonly IUserManager _userManager; - private readonly Dictionary> _changedItems = new(); + private readonly Dictionary> _changedItems = []; private readonly Lock _syncLock = new(); private Timer? _updateTimer; + private int _changedItemCount; /// /// Initializes a new instance of the class. @@ -69,50 +71,64 @@ namespace Emby.Server.Implementations.EntryPoints lock (_syncLock) { - if (_updateTimer is null) - { - _updateTimer = new Timer( - UpdateTimerCallback, - null, - UpdateDuration, - Timeout.Infinite); - } - else - { - _updateTimer.Change(UpdateDuration, Timeout.Infinite); - } + // The window runs from the first change of a batch and is never extended, so a stream + // of changes that never pauses - a library scan - still closes its batches instead of + // holding every item it touched alive until the stream stops. + _updateTimer ??= new Timer( + UpdateTimerCallback, + null, + UpdateDuration, + Timeout.Infinite); - if (!_changedItems.TryGetValue(e.UserId, out List? keys)) + if (!_changedItems.TryGetValue(e.UserId, out Dictionary? keys)) { - keys = new List(); + keys = []; _changedItems[e.UserId] = keys; } - keys.Add(e.Item); - var baseItem = e.Item; // Go up one level for indicators if (baseItem is not null) { + Track(keys, baseItem); + var parent = baseItem.GetOwner() ?? baseItem.GetParent(); if (parent is not null) { - keys.Add(parent); + Track(keys, parent); } } + + // A window long enough to cover a burst still has to give way once the batch is + // large enough to be worth sending on its own. + if (_changedItemCount >= MaxBatchSize) + { + _updateTimer.Change(0, Timeout.Infinite); + } + } + } + + private void Track(Dictionary keys, BaseItem item) + { + var before = keys.Count; + keys[item.Id] = item; + + if (keys.Count != before) + { + _changedItemCount++; } } private async void UpdateTimerCallback(object? state) { - List>> changes; + List>> changes; lock (_syncLock) { - // Remove dupes in case some were saved multiple times changes = _changedItems.ToList(); _changedItems.Clear(); + _changedItemCount = 0; if (_updateTimer is not null) { @@ -121,17 +137,22 @@ namespace Emby.Server.Implementations.EntryPoints } } + if (changes.Count == 0) + { + return; + } + foreach (var (userId, changedItems) in changes) { await _sessionManager.SendMessageToUserSessions( [userId], SessionMessageType.UserDataChanged, - () => GetUserDataChangeInfo(userId, changedItems), + () => GetUserDataChangeInfo(userId, changedItems.Values), default).ConfigureAwait(false); } } - private UserDataChangeInfo GetUserDataChangeInfo(Guid userId, List changedItems) + private UserDataChangeInfo GetUserDataChangeInfo(Guid userId, IEnumerable changedItems) { var user = _userManager.GetUserById(userId) ?? throw new ArgumentException("Invalid user ID", nameof(userId)); @@ -140,7 +161,6 @@ namespace Emby.Server.Implementations.EntryPoints { UserId = userId, UserDataList = changedItems - .DistinctBy(x => x.Id) .Select(i => { var dto = _userDataManager.GetUserDataDto(i, user); diff --git a/tests/Jellyfin.Server.Implementations.Tests/EntryPoints/LibraryChangedNotifierTests.cs b/tests/Jellyfin.Server.Implementations.Tests/EntryPoints/LibraryChangedNotifierTests.cs new file mode 100644 index 0000000000..cdb261de8d --- /dev/null +++ b/tests/Jellyfin.Server.Implementations.Tests/EntryPoints/LibraryChangedNotifierTests.cs @@ -0,0 +1,123 @@ +using System; +using System.Collections.Generic; +using System.Diagnostics; +using System.Threading; +using System.Threading.Tasks; +using Emby.Server.Implementations.EntryPoints; +using MediaBrowser.Controller.Configuration; +using MediaBrowser.Controller.Entities; +using MediaBrowser.Controller.Library; +using MediaBrowser.Controller.Providers; +using MediaBrowser.Controller.Session; +using MediaBrowser.Model.Configuration; +using Microsoft.Extensions.Logging.Abstractions; +using Moq; +using Xunit; + +namespace Jellyfin.Server.Implementations.Tests.EntryPoints; + +public class LibraryChangedNotifierTests +{ + // How long a test waits for the notifier's timer callback to run. Generous: the assertions are + // about a batch being sent at all, not about how promptly. + private static readonly TimeSpan _flushTimeout = TimeSpan.FromSeconds(15); + + private readonly Mock _libraryManager = new(); + private readonly Mock _configurationManager = new(); + private readonly Mock _sessionManager = new(); + private readonly Mock _userManager = new(); + private readonly Mock _providerManager = new(); + private readonly ServerConfiguration _configuration = new(); + + private int _flushCount; + + public LibraryChangedNotifierTests() + { + _configurationManager.SetupGet(e => e.Configuration).Returns(_configuration); + + // Reading the session list is the first thing a flush does, so it stands in for "a batch was + // sent" without having to mock a whole user library behind it. + _sessionManager.SetupGet(e => e.Sessions) + .Returns(() => + { + Interlocked.Increment(ref _flushCount); + return []; + }); + } + + [Fact] + public async Task OnLibraryItemUpdated_BatchSizeCapReached_SendsWithoutWaitingForWindow() + { + // Long enough that only the size cap can close the batch. + _configuration.LibraryUpdateDuration = 3600; + + var notifier = CreateNotifier(); + await notifier.StartAsync(TestContext.Current.CancellationToken); + + for (var i = 0; i < LibraryChangedNotifier.MaxBatchSize; i++) + { + RaiseItemUpdated(); + } + + Assert.True(await WaitForFlushAsync(1), "The batch was not sent once it hit the size cap."); + + await notifier.StopAsync(TestContext.Current.CancellationToken); + notifier.Dispose(); + } + + [Fact] + public async Task OnLibraryItemUpdated_ChangesNeverPause_StillSendsOnTheWindow() + { + // A scan changes items continuously. The window must run from the first change of a batch, or + // the batch never closes and holds every item it named alive for the length of the scan. + _configuration.LibraryUpdateDuration = 1; + + var notifier = CreateNotifier(); + await notifier.StartAsync(TestContext.Current.CancellationToken); + + var stopwatch = Stopwatch.StartNew(); + while (stopwatch.Elapsed < _flushTimeout && Volatile.Read(ref _flushCount) == 0) + { + // Well below the window, and well below the size cap over the whole loop. + RaiseItemUpdated(); + await Task.Delay(25, TestContext.Current.CancellationToken); + } + + Assert.True(Volatile.Read(ref _flushCount) > 0, "The batch was never sent while changes kept arriving."); + + await notifier.StopAsync(TestContext.Current.CancellationToken); + notifier.Dispose(); + } + + private LibraryChangedNotifier CreateNotifier() + => new( + _libraryManager.Object, + _configurationManager.Object, + _sessionManager.Object, + _userManager.Object, + NullLogger.Instance, + _providerManager.Object); + + // A folder passes the notifier's item filter without needing any of BaseItem's static services. + private void RaiseItemUpdated() + => _libraryManager.Raise( + e => e.ItemUpdated += null, + _libraryManager.Object, + new ItemChangeEventArgs { Item = new Folder { Id = Guid.NewGuid() } }); + + private async Task WaitForFlushAsync(int expected) + { + var stopwatch = Stopwatch.StartNew(); + while (stopwatch.Elapsed < _flushTimeout) + { + if (Volatile.Read(ref _flushCount) >= expected) + { + return true; + } + + await Task.Delay(25, TestContext.Current.CancellationToken); + } + + return false; + } +} diff --git a/tests/Jellyfin.Server.Implementations.Tests/EntryPoints/UserDataChangeNotifierTests.cs b/tests/Jellyfin.Server.Implementations.Tests/EntryPoints/UserDataChangeNotifierTests.cs new file mode 100644 index 0000000000..0274398f89 --- /dev/null +++ b/tests/Jellyfin.Server.Implementations.Tests/EntryPoints/UserDataChangeNotifierTests.cs @@ -0,0 +1,78 @@ +using System; +using System.Diagnostics; +using System.Threading; +using System.Threading.Tasks; +using Emby.Server.Implementations.EntryPoints; +using MediaBrowser.Controller.Entities; +using MediaBrowser.Controller.Library; +using MediaBrowser.Controller.Session; +using MediaBrowser.Model.Entities; +using MediaBrowser.Model.Session; +using Moq; +using Xunit; + +namespace Jellyfin.Server.Implementations.Tests.EntryPoints; + +public class UserDataChangeNotifierTests +{ + // How long a test waits for the notifier's timer callback to run. Generous: the assertions are + // about a batch being sent at all, not about how promptly. + private static readonly TimeSpan _flushTimeout = TimeSpan.FromSeconds(15); + + private readonly Mock _userDataManager = new(); + private readonly Mock _sessionManager = new(); + private readonly Mock _userManager = new(); + + private int _flushCount; + + public UserDataChangeNotifierTests() + { + _sessionManager + .Setup(e => e.SendMessageToUserSessions( + It.IsAny>(), + SessionMessageType.UserDataChanged, + It.IsAny>(), + It.IsAny())) + .Callback(() => Interlocked.Increment(ref _flushCount)) + .Returns(Task.CompletedTask); + } + + [Fact] + public async Task OnUserDataSaved_ChangesNeverPause_StillSendsOnTheWindow() + { + // A scan changes user data continuously. The window must run from the first change of a batch, + // or the batch never closes and holds every item it named alive for the length of the scan. + var notifier = CreateNotifier(); + await notifier.StartAsync(TestContext.Current.CancellationToken); + + var userId = Guid.NewGuid(); + var stopwatch = Stopwatch.StartNew(); + while (stopwatch.Elapsed < _flushTimeout && Volatile.Read(ref _flushCount) == 0) + { + // Well below the window, and well below the size cap over the whole loop. + RaiseUserDataSaved(userId); + await Task.Delay(25, TestContext.Current.CancellationToken); + } + + Assert.True(Volatile.Read(ref _flushCount) > 0, "The batch was never sent while changes kept arriving."); + + await notifier.StopAsync(TestContext.Current.CancellationToken); + notifier.Dispose(); + } + + private UserDataChangeNotifier CreateNotifier() + => new(_userDataManager.Object, _sessionManager.Object, _userManager.Object); + + // A folder needs none of BaseItem's static services, and PlaybackProgress is the one reason the + // notifier ignores outright. + private void RaiseUserDataSaved(Guid userId) + => _userDataManager.Raise( + e => e.UserDataSaved += null, + _userDataManager.Object, + new UserDataSaveEventArgs + { + UserId = userId, + SaveReason = UserDataSaveReason.UpdateUserRating, + Item = new Folder { Id = Guid.NewGuid() } + }); +} From c56e14d8fb559abcc73fc7c2c83533cd32cb5320 Mon Sep 17 00:00:00 2001 From: Shadowghost Date: Tue, 1 Sep 2026 20:47:39 +0200 Subject: [PATCH 05/17] Keep a media process and its exit state usable by the caller that started it --- .../Encoder/MediaEncoder.cs | 86 +++++++++++---- .../Encoder/ProcessWrapperTests.cs | 103 ++++++++++++++++++ 2 files changed, 170 insertions(+), 19 deletions(-) create mode 100644 tests/Jellyfin.MediaEncoding.Tests/Encoder/ProcessWrapperTests.cs diff --git a/MediaBrowser.MediaEncoding/Encoder/MediaEncoder.cs b/MediaBrowser.MediaEncoding/Encoder/MediaEncoder.cs index 0ddd378352..f64fd73763 100644 --- a/MediaBrowser.MediaEncoding/Encoder/MediaEncoder.cs +++ b/MediaBrowser.MediaEncoding/Encoder/MediaEncoder.cs @@ -1152,6 +1152,11 @@ namespace MediaBrowser.MediaEncoding.Encoder { process.Process.PriorityClass = ProcessPriorityClass.BelowNormal; } + catch (InvalidOperationException) + { + // The process finished before its priority could be lowered. That says nothing + // about whether the platform allows it, so keep the capability for the next one. + } catch (Exception ex) { _canSetProcessPriority = false; @@ -1361,12 +1366,20 @@ namespace MediaBrowser.MediaEncoding.Encoder return _configurationManager.GetEncodingOptions().EnableSubtitleExtraction; } - private sealed class ProcessWrapper : IDisposable + internal sealed class ProcessWrapper : IDisposable { private readonly MediaEncoder _mediaEncoder; + // The exit event is raised on the thread pool, so it writes the state below while the + // caller that started the process is reading it. + private readonly Lock _exitLock = new(); + private bool _disposed = false; + private bool _hasExited; + + private int? _exitCode; + public ProcessWrapper(Process process, MediaEncoder mediaEncoder) { Process = process; @@ -1376,49 +1389,84 @@ namespace MediaBrowser.MediaEncoding.Encoder public Process Process { get; } - public bool HasExited { get; private set; } + // The exit event can lag behind the wait that returned, so ask the process rather than + // report one that has exited as still running. + public bool HasExited => ReadExitState().HasExited; - public int? ExitCode { get; private set; } + // As above: rather than report no exit code for a process that has one. + public int? ExitCode => ReadExitState().ExitCode; + + private (bool HasExited, int? ExitCode) ReadExitState() + { + lock (_exitLock) + { + if (!_hasExited && !_disposed) + { + try + { + if (Process.HasExited) + { + _hasExited = true; + _exitCode = Process.ExitCode; + } + } + catch (InvalidOperationException) + { + // No process is associated with this object, or it was disposed from + // under us - ObjectDisposedException derives from this one. + } + } + + return (_hasExited, _exitCode); + } + } private void OnProcessExited(object sender, EventArgs e) { var process = (Process)sender; - HasExited = true; + lock (_exitLock) + { + _hasExited = true; - try - { - ExitCode = process.ExitCode; - } - catch - { + try + { + _exitCode = process.ExitCode; + } + catch + { + } } - DisposeProcess(process); + // Only stop tracking it. The caller that started the process still holds it to read + // its output and its exit code, so disposing it here handed whoever was quickest to + // exit - an ffprobe on a file it rejects outright - an ObjectDisposedException. + Untrack(); } - private void DisposeProcess(Process process) + private void Untrack() { lock (_mediaEncoder._runningProcessesLock) { _mediaEncoder._runningProcesses.Remove(this); } - - process.Dispose(); } public void Dispose() { - if (!_disposed) + lock (_exitLock) { - if (Process is not null) + if (_disposed) { - Process.Exited -= OnProcessExited; - DisposeProcess(Process); + return; } + + _disposed = true; } - _disposed = true; + Process.Exited -= OnProcessExited; + Untrack(); + Process.Dispose(); } } } diff --git a/tests/Jellyfin.MediaEncoding.Tests/Encoder/ProcessWrapperTests.cs b/tests/Jellyfin.MediaEncoding.Tests/Encoder/ProcessWrapperTests.cs new file mode 100644 index 0000000000..141164815c --- /dev/null +++ b/tests/Jellyfin.MediaEncoding.Tests/Encoder/ProcessWrapperTests.cs @@ -0,0 +1,103 @@ +using System; +using System.Diagnostics; +using System.Threading; +using System.Threading.Tasks; +using MediaBrowser.Controller.Configuration; +using MediaBrowser.MediaEncoding.Encoder; +using MediaBrowser.Model.Globalization; +using MediaBrowser.Model.IO; +using MediaBrowser.Model.MediaInfo; +using Microsoft.Extensions.Configuration; +using Microsoft.Extensions.Logging; +using Moq; +using Xunit; + +namespace Jellyfin.MediaEncoding.Tests.Encoder; + +public class ProcessWrapperTests +{ + [Fact] + public async Task ExitedProcess_StaysUsableForTheCallerThatStartedIt() + { + using var process = CreateProcess(); + using var exitHandled = new ManualResetEventSlim(false); + + using (var wrapper = new MediaEncoder.ProcessWrapper(process, CreateEncoder())) + { + // Subscribed after the wrapper, so by the time this is set the wrapper's own handler has + // already run: whatever it does to the process has happened. + process.Exited += (_, _) => exitHandled.Set(); + + process.Start(); + await process.WaitForExitAsync(TestContext.Current.CancellationToken).ConfigureAwait(true); + + Assert.True(exitHandled.Wait(TimeSpan.FromSeconds(15), TestContext.Current.CancellationToken), "The process never raised Exited."); + + // The caller still owns the process here. Disposing it from the exit handler handed + // whoever exited quickest an ObjectDisposedException out of these three lines. + var output = await process.StandardOutput.ReadToEndAsync(TestContext.Current.CancellationToken).ConfigureAwait(true); + Assert.Equal("jellyfin", output.Trim()); + + Assert.True(wrapper.HasExited); + Assert.Equal(3, wrapper.ExitCode); + } + } + + [Fact] + public async Task ExitState_IsReadableBeforeTheExitEventArrives() + { + using var process = CreateProcess(); + + using (var wrapper = new MediaEncoder.ProcessWrapper(process, CreateEncoder())) + { + process.Start(); + await process.WaitForExitAsync(TestContext.Current.CancellationToken).ConfigureAwait(true); + + // The exit event is raised on the thread pool and can lag behind the wait that just + // returned, so neither of these may depend on it having arrived. + Assert.True(wrapper.HasExited); + Assert.Equal(3, wrapper.ExitCode); + } + } + + [Fact] + public async Task ExitCode_SurvivesDisposal() + { + using var process = CreateProcess(); + var wrapper = new MediaEncoder.ProcessWrapper(process, CreateEncoder()); + + process.Start(); + await process.WaitForExitAsync(TestContext.Current.CancellationToken).ConfigureAwait(true); + + var exitCode = wrapper.ExitCode; + wrapper.Dispose(); + + Assert.Equal(exitCode, wrapper.ExitCode); + Assert.True(wrapper.HasExited); + } + + private static MediaEncoder CreateEncoder() + => new( + Mock.Of>(), + Mock.Of(), + Mock.Of(), + Mock.Of(), + Mock.Of(), + new ConfigurationBuilder().Build(), + Mock.Of()); + + // Writes to stdout and exits immediately with a non-zero code, standing in for the ffprobe that + // rejects a file outright - the process that used to win the race against its own caller. + private static Process CreateProcess() + { + var startInfo = OperatingSystem.IsWindows() + ? new ProcessStartInfo("cmd.exe", "/c echo jellyfin & exit 3") + : new ProcessStartInfo("/bin/sh", "-c \"printf 'jellyfin\\n'; exit 3\""); + + startInfo.CreateNoWindow = true; + startInfo.UseShellExecute = false; + startInfo.RedirectStandardOutput = true; + + return new Process { StartInfo = startInfo, EnableRaisingEvents = true }; + } +} From ccdc69e3b012381f49900aa930d1dc876096cfe6 Mon Sep 17 00:00:00 2001 From: Shadowghost Date: Tue, 1 Sep 2026 20:47:39 +0200 Subject: [PATCH 06/17] Treat an item deleted mid-save as a no-op when saving its images --- .../Item/ItemPersistenceService.cs | 29 ++++--- .../ItemPersistenceServiceSaveImagesTests.cs | 77 +++++++++++++++++++ .../Item/SqliteDbTestFixture.cs | 3 + 3 files changed, 100 insertions(+), 9 deletions(-) create mode 100644 tests/Jellyfin.Server.Implementations.Tests/Item/ItemPersistenceServiceSaveImagesTests.cs diff --git a/Jellyfin.Server.Implementations/Item/ItemPersistenceService.cs b/Jellyfin.Server.Implementations/Item/ItemPersistenceService.cs index efff3457a3..c8672e189b 100644 --- a/Jellyfin.Server.Implementations/Item/ItemPersistenceService.cs +++ b/Jellyfin.Server.Implementations/Item/ItemPersistenceService.cs @@ -176,14 +176,6 @@ public class ItemPersistenceService : IItemPersistenceService var context = await _dbProvider.CreateDbContextAsync(cancellationToken).ConfigureAwait(false); await using (context.ConfigureAwait(false)) { - if (!await context.BaseItems - .AnyAsync(bi => bi.Id == item.Id, cancellationToken) - .ConfigureAwait(false)) - { - _logger.LogWarning("Unable to save ImageInfo for non existing BaseItem"); - return; - } - await context.BaseItemImageInfos .Where(e => e.ItemId == item.Id) .ExecuteDeleteAsync(cancellationToken) @@ -193,7 +185,26 @@ public class ItemPersistenceService : IItemPersistenceService .AddRangeAsync(images, cancellationToken) .ConfigureAwait(false); - await context.SaveChangesAsync(cancellationToken).ConfigureAwait(false); + try + { + await context.SaveChangesAsync(cancellationToken).ConfigureAwait(false); + } + catch (DbUpdateException) + { + // Checking that the item exists before writing leaves a gap a scan can delete it + // through, turning the insert into a foreign key violation that fails the whole + // refresh instead of the no-op intended here. Let the insert be the check: it is the + // only point at which the answer cannot go stale. Nothing is orphaned by the delete + // above, because deleting the item cascades to its images anyway. + if (await context.BaseItems + .AnyAsync(bi => bi.Id == item.Id, cancellationToken) + .ConfigureAwait(false)) + { + throw; + } + + _logger.LogWarning("Unable to save ImageInfo for non existing BaseItem {ItemId}", item.Id); + } } } diff --git a/tests/Jellyfin.Server.Implementations.Tests/Item/ItemPersistenceServiceSaveImagesTests.cs b/tests/Jellyfin.Server.Implementations.Tests/Item/ItemPersistenceServiceSaveImagesTests.cs new file mode 100644 index 0000000000..7997c6d771 --- /dev/null +++ b/tests/Jellyfin.Server.Implementations.Tests/Item/ItemPersistenceServiceSaveImagesTests.cs @@ -0,0 +1,77 @@ +using System; +using System.Linq; +using System.Threading.Tasks; +using Jellyfin.Database.Implementations.Entities; +using Jellyfin.Server.Implementations.Item; +using MediaBrowser.Controller; +using MediaBrowser.Controller.Entities; +using MediaBrowser.Model.Entities; +using Microsoft.EntityFrameworkCore; +using Microsoft.Extensions.Logging.Abstractions; +using Moq; +using Xunit; + +namespace Jellyfin.Server.Implementations.Tests.Item; + +public class ItemPersistenceServiceSaveImagesTests : SqliteDbTestFixture +{ + private readonly ItemPersistenceService _service; + + public ItemPersistenceServiceSaveImagesTests() + { + _service = new ItemPersistenceService( + CreateDbContextFactory(), + Mock.Of(), + NullLogger.Instance); + } + + [Fact] + public async Task SaveImagesAsync_ReplacesThePreviousImages() + { + var itemId = Guid.NewGuid(); + Seed(itemId); + + await _service.SaveImagesAsync(CreateItem(itemId, "/first.jpg"), TestContext.Current.CancellationToken); + await _service.SaveImagesAsync(CreateItem(itemId, "/second.jpg"), TestContext.Current.CancellationToken); + + using var context = CreateDbContext(); + var paths = context.BaseItemImageInfos + .Where(e => e.ItemId.Equals(itemId)) + .Select(e => e.Path) + .ToList(); + + Assert.Equal(["/second.jpg"], paths); + } + + [Fact] + public async Task SaveImagesAsync_ItemDeletedFromUnderIt_IsANoOp() + { + // A scan can delete the item between the refresh reading it and the images being written. That + // must not fail the whole refresh, and must not leave the images of an item that is gone. + var itemId = Guid.NewGuid(); + + await _service.SaveImagesAsync(CreateItem(itemId, "/gone.jpg"), TestContext.Current.CancellationToken); + + using var context = CreateDbContext(); + Assert.Empty(context.BaseItemImageInfos.Where(e => e.ItemId.Equals(itemId))); + } + + private static BaseItem CreateItem(Guid itemId, string imagePath) + => new Folder + { + Id = itemId, + ImageInfos = [new ItemImageInfo { Path = imagePath, Type = ImageType.Primary }] + }; + + private void Seed(Guid itemId) + { + using var context = CreateDbContext(); + context.BaseItems.Add(new BaseItemEntity + { + Id = itemId, + Type = "Folder", + IsFolder = true + }); + context.SaveChanges(); + } +} diff --git a/tests/Jellyfin.Server.Implementations.Tests/Item/SqliteDbTestFixture.cs b/tests/Jellyfin.Server.Implementations.Tests/Item/SqliteDbTestFixture.cs index 87efa8fea5..cfc9c9496c 100644 --- a/tests/Jellyfin.Server.Implementations.Tests/Item/SqliteDbTestFixture.cs +++ b/tests/Jellyfin.Server.Implementations.Tests/Item/SqliteDbTestFixture.cs @@ -1,4 +1,5 @@ using System; +using System.Threading; using Emby.Server.Implementations.Data; using Jellyfin.Database.Implementations; using Jellyfin.Database.Implementations.Locking; @@ -58,6 +59,8 @@ public abstract class SqliteDbTestFixture : IDisposable { var factory = new Mock>(); factory.Setup(f => f.CreateDbContext()).Returns(CreateDbContext); + factory.Setup(f => f.CreateDbContextAsync(It.IsAny())) + .ReturnsAsync(CreateDbContext); return factory.Object; } From 8eb4964599d8dcb9b6bbd178b4794d1a69504368 Mon Sep 17 00:00:00 2001 From: Shadowghost Date: Wed, 2 Sep 2026 09:29:48 +0200 Subject: [PATCH 07/17] Never treat a manifest container as an audio codec or a direct play target --- .../MediaEncoding/EncodingHelper.cs | 6 +- MediaBrowser.Model/Dlna/StreamBuilder.cs | 10 ++ .../TunerHosts/M3UTunerHost.cs | 26 ++++- .../EncodingHelperInferAudioCodecTests.cs | 40 +++++++ .../M3UTunerHostTests.cs | 72 ++++++++++++ .../StreamBuilderManifestContainerTests.cs | 104 ++++++++++++++++++ 6 files changed, 256 insertions(+), 2 deletions(-) create mode 100644 tests/Jellyfin.Controller.Tests/MediaEncoding/EncodingHelperInferAudioCodecTests.cs create mode 100644 tests/Jellyfin.LiveTv.Tests/M3UTunerHostTests.cs create mode 100644 tests/Jellyfin.Model.Tests/Dlna/StreamBuilderManifestContainerTests.cs diff --git a/MediaBrowser.Controller/MediaEncoding/EncodingHelper.cs b/MediaBrowser.Controller/MediaEncoding/EncodingHelper.cs index 10c21ee03c..274e82d823 100644 --- a/MediaBrowser.Controller/MediaEncoding/EncodingHelper.cs +++ b/MediaBrowser.Controller/MediaEncoding/EncodingHelper.cs @@ -695,7 +695,11 @@ namespace MediaBrowser.Controller.MediaEncoding "ogg" or "oga" or "ogv" or "webm" or "webma" => "opus", "m4a" or "m4b" or "mp4" or "mov" or "mkv" or "mka" => "aac", "ts" or "avi" or "flv" or "f4v" or "swf" => "mp3", - _ => inferredCodec + // Containers that share their name with the codec they carry. + "aac" or "ac3" or "alac" or "dts" or "eac3" or "flac" or "mp2" or "mp3" or "opus" or "truehd" or "vorbis" => inferredCodec, + // Anything else - manifests such as m3u8/mpd in particular - names a container that + // is not an audio codec. Never hand that name to ffmpeg as an encoder. + _ => "aac" }; } diff --git a/MediaBrowser.Model/Dlna/StreamBuilder.cs b/MediaBrowser.Model/Dlna/StreamBuilder.cs index ab8d5dd5b2..4799ed5410 100644 --- a/MediaBrowser.Model/Dlna/StreamBuilder.cs +++ b/MediaBrowser.Model/Dlna/StreamBuilder.cs @@ -26,6 +26,8 @@ namespace MediaBrowser.Model.Dlna internal const TranscodeReason VideoReasons = TranscodeReason.VideoCodecNotSupported | VideoCodecReasons; internal const TranscodeReason DirectStreamReasons = AudioReasons | TranscodeReason.ContainerNotSupported | TranscodeReason.VideoCodecTagNotSupported; + private const string ManifestContainers = "hls,applehttp,dash"; + private readonly ILogger _logger; private readonly ITranscoderSupport _transcoderSupport; private static readonly string[] _supportedHlsVideoCodecs = ["h264", "hevc", "vp9", "av1"]; @@ -718,6 +720,14 @@ namespace MediaBrowser.Model.Dlna isEligibleForDirectPlay = false; } + // A manifest is not a byte stream, so it cannot be handed to the client as one. The variant + // and segment URIs inside it are relative to the origin and do not resolve against the + // Jellyfin url the client would fetch it from. + if (ContainerHelper.ContainsContainer(ManifestContainers, item.Container)) + { + isEligibleForDirectPlay = false; + } + if (bitrateLimitExceeded) { transcodeReasons = TranscodeReason.ContainerBitrateExceedsLimit; diff --git a/src/Jellyfin.LiveTv/TunerHosts/M3UTunerHost.cs b/src/Jellyfin.LiveTv/TunerHosts/M3UTunerHost.cs index fb606be0ef..902ca76af8 100644 --- a/src/Jellyfin.LiveTv/TunerHosts/M3UTunerHost.cs +++ b/src/Jellyfin.LiveTv/TunerHosts/M3UTunerHost.cs @@ -32,6 +32,7 @@ namespace Jellyfin.LiveTv.TunerHosts { private static readonly string[] _mimeTypesCanShareHttpStream = ["video/MP2T"]; private static readonly string[] _extensionsCanShareHttpStream = [".ts", ".tsv", ".m2t"]; + private static readonly string[] _manifestExtensions = [".m3u8", ".m3u", ".mpd"]; private readonly IHttpClientFactory _httpClientFactory; private readonly IServerApplicationHost _appHost; @@ -151,11 +152,20 @@ namespace Jellyfin.LiveTv.TunerHosts var protocol = _mediaSourceManager.GetPathProtocol(path); var isRemote = true; - if (Uri.TryCreate(path, UriKind.Absolute, out var uri)) + Uri.TryCreate(path, UriKind.Absolute, out var uri); + if (uri is not null) { isRemote = !_networkManager.IsInLocalNetwork(uri.Host); } + // A manifest is not a byte stream. Serving one directly hands the client a playlist whose + // variant and segment URIs are relative to the origin, and those do not resolve against the + // Jellyfin url the client fetched it from. Remux or transcode these instead. + if (IsManifest(path, uri)) + { + supportsDirectPlay = false; + } + var httpHeaders = new Dictionary(); if (protocol == MediaProtocol.Http) @@ -210,6 +220,20 @@ namespace Jellyfin.LiveTv.TunerHosts return mediaSource; } + /// + /// Determines whether a channel path points at an HLS or DASH manifest rather than at a byte stream. + /// + /// The channel path. + /// The channel path parsed as an absolute uri, or null if it is not one. + /// true if the path names a streaming manifest. + private static bool IsManifest(string path, Uri uri) + { + // Use the uri path when there is one so that a query string does not hide the extension. + var extension = Path.GetExtension(uri is null ? path : uri.AbsolutePath); + + return _manifestExtensions.Contains(extension, StringComparison.OrdinalIgnoreCase); + } + public Task> DiscoverDevices(int discoveryDurationMs, CancellationToken cancellationToken) { return Task.FromResult(new List()); diff --git a/tests/Jellyfin.Controller.Tests/MediaEncoding/EncodingHelperInferAudioCodecTests.cs b/tests/Jellyfin.Controller.Tests/MediaEncoding/EncodingHelperInferAudioCodecTests.cs new file mode 100644 index 0000000000..586db2dd50 --- /dev/null +++ b/tests/Jellyfin.Controller.Tests/MediaEncoding/EncodingHelperInferAudioCodecTests.cs @@ -0,0 +1,40 @@ +using MediaBrowser.Common.Configuration; +using MediaBrowser.Controller.IO; +using MediaBrowser.Controller.MediaEncoding; +using Moq; +using Xunit; +using IConfiguration = Microsoft.Extensions.Configuration.IConfiguration; + +namespace Jellyfin.Controller.Tests.MediaEncoding; + +public class EncodingHelperInferAudioCodecTests +{ + [Theory] + // Manifests and other containers that carry no inferable audio codec. + [InlineData("m3u8", "aac")] + [InlineData("mpd", "aac")] + [InlineData("wtv", "aac")] + [InlineData("", "aac")] + // Containers with a well known audio codec. + [InlineData("mp4", "aac")] + [InlineData("mkv", "aac")] + [InlineData("webm", "opus")] + [InlineData("ts", "mp3")] + // Containers named after the codec they carry. + [InlineData("flac", "flac")] + [InlineData("opus", "opus")] + [InlineData("ac3", "ac3")] + public void InferAudioCodec_ReturnsAnAudioCodec(string container, string expected) + { + Assert.Equal(expected, Create().InferAudioCodec(container)); + } + + private static EncodingHelper Create() + => new( + Mock.Of(), + Mock.Of(), + Mock.Of(), + Mock.Of(), + Mock.Of(), + Mock.Of()); +} diff --git a/tests/Jellyfin.LiveTv.Tests/M3UTunerHostTests.cs b/tests/Jellyfin.LiveTv.Tests/M3UTunerHostTests.cs new file mode 100644 index 0000000000..4487a5ff2b --- /dev/null +++ b/tests/Jellyfin.LiveTv.Tests/M3UTunerHostTests.cs @@ -0,0 +1,72 @@ +using System.Collections.Generic; +using System.Net.Http; +using System.Threading; +using System.Threading.Tasks; +using Jellyfin.LiveTv.TunerHosts; +using MediaBrowser.Common.Net; +using MediaBrowser.Controller; +using MediaBrowser.Controller.Configuration; +using MediaBrowser.Controller.Library; +using MediaBrowser.Controller.LiveTv; +using MediaBrowser.Model.Dto; +using MediaBrowser.Model.IO; +using MediaBrowser.Model.LiveTv; +using MediaBrowser.Model.MediaInfo; +using Microsoft.Extensions.Logging; +using Moq; +using Xunit; + +namespace Jellyfin.LiveTv.Tests +{ + public class M3UTunerHostTests + { + [Theory] + // A manifest is not a byte stream, so it must never be offered for direct play. + [InlineData("http://example.com/live/1234.m3u8", false)] + [InlineData("http://example.com/live/1234.m3u8?token=abc", false)] + [InlineData("http://example.com/live/1234.mpd", false)] + // Byte streams are unaffected. + [InlineData("http://example.com/live/1234.ts", true)] + [InlineData("http://example.com/live/1234", true)] + public async Task GetChannelStreamMediaSources_ManifestPath_DisablesDirectPlay(string path, bool expectDirectPlay) + { + var mediaSourceManager = new Mock(); + mediaSourceManager.Setup(x => x.GetPathProtocol(It.IsAny())).Returns(MediaProtocol.Http); + + var host = new TestableM3UTunerHost( + Mock.Of(), + mediaSourceManager.Object, + Mock.Of>(), + Mock.Of(), + Mock.Of(), + Mock.Of(), + Mock.Of(), + Mock.Of()); + + var sources = await host.GetMediaSources( + new TunerHostInfo { TunerCount = 0, EnableStreamLooping = false }, + new ChannelInfo { Path = path }); + + Assert.Equal(expectDirectPlay, sources[0].SupportsDirectPlay); + } + + private sealed class TestableM3UTunerHost : M3UTunerHost + { + public TestableM3UTunerHost( + IServerConfigurationManager config, + IMediaSourceManager mediaSourceManager, + ILogger logger, + IFileSystem fileSystem, + IHttpClientFactory httpClientFactory, + IServerApplicationHost appHost, + INetworkManager networkManager, + IStreamHelper streamHelper) + : base(config, mediaSourceManager, logger, fileSystem, httpClientFactory, appHost, networkManager, streamHelper) + { + } + + public Task> GetMediaSources(TunerHostInfo tuner, ChannelInfo channel) + => GetChannelStreamMediaSources(tuner, channel, CancellationToken.None); + } + } +} diff --git a/tests/Jellyfin.Model.Tests/Dlna/StreamBuilderManifestContainerTests.cs b/tests/Jellyfin.Model.Tests/Dlna/StreamBuilderManifestContainerTests.cs new file mode 100644 index 0000000000..dfd1eb2e85 --- /dev/null +++ b/tests/Jellyfin.Model.Tests/Dlna/StreamBuilderManifestContainerTests.cs @@ -0,0 +1,104 @@ +using System; +using Jellyfin.Data.Enums; +using MediaBrowser.Model.Dlna; +using MediaBrowser.Model.Dto; +using MediaBrowser.Model.Entities; +using MediaBrowser.Model.MediaInfo; +using MediaBrowser.Model.Session; +using Microsoft.Extensions.Logging.Abstractions; +using Moq; +using Xunit; + +namespace Jellyfin.Model.Tests.Dlna; + +public class StreamBuilderManifestContainerTests +{ + [Theory] + // A manifest describes a stream instead of carrying one, so it can never be direct played, + // even when the client claims to support the container. + [InlineData("hls")] + [InlineData("hls,applehttp")] + [InlineData("applehttp")] + [InlineData("dash")] + public void GetOptimalVideoStream_ManifestContainer_DoesNotDirectPlay(string container) + { + var streamInfo = BuildFor(container); + + Assert.NotNull(streamInfo); + Assert.Equal(PlayMethod.Transcode, streamInfo.PlayMethod); + } + + [Fact] + public void GetOptimalVideoStream_ByteStreamContainer_StillDirectPlays() + { + var streamInfo = BuildFor("mp4"); + + Assert.NotNull(streamInfo); + Assert.Equal(PlayMethod.DirectPlay, streamInfo.PlayMethod); + } + + private static StreamInfo? BuildFor(string container) + { + var mediaSource = new MediaSourceInfo + { + Id = "test-source", + Path = "http://example.com/live/channel", + Protocol = MediaProtocol.Http, + Container = container, + SupportsDirectPlay = true, + SupportsDirectStream = true, + SupportsTranscoding = true, + IsInfiniteStream = true, + IsRemote = true, + MediaStreams = + [ + new MediaStream { Type = MediaStreamType.Video, Index = 0, Codec = "h264" }, + new MediaStream { Type = MediaStreamType.Audio, Index = 1, Codec = "aac" } + ] + }; + + var profile = new DeviceProfile + { + Name = "Manifest aware client", + DirectPlayProfiles = + [ + new DirectPlayProfile + { + Type = DlnaProfileType.Video, + Container = "mp4,hls,applehttp,dash", + VideoCodec = "h264", + AudioCodec = "aac" + } + ], + TranscodingProfiles = + [ + new TranscodingProfile + { + Type = DlnaProfileType.Video, + Context = EncodingContext.Streaming, + Protocol = MediaStreamProtocol.hls, + Container = "ts", + VideoCodec = "h264", + AudioCodec = "aac" + } + ] + }; + + var options = new MediaOptions + { + ItemId = new Guid("11D229B7-2D48-4B95-9F9B-49F6AB75E613"), + MediaSourceId = mediaSource.Id, + MediaSources = [mediaSource], + DeviceId = "test-deviceId", + Profile = profile, + AllowAudioStreamCopy = true, + AllowVideoStreamCopy = true, + EnableDirectStream = false // This is disabled in server + }; + + var transcodeSupport = new Mock(); + + return new StreamBuilder(transcodeSupport.Object, new NullLogger()) + .GetOptimalVideoStream(options); + } +} From 0b5bbb528af08d950bc9887b5a1114888820688b Mon Sep 17 00:00:00 2001 From: Shadowghost Date: Fri, 4 Sep 2026 18:57:54 +0200 Subject: [PATCH 08/17] Optimize database after running migrations --- .../Tasks/OptimizeDatabaseTask.cs | 4 +-- .../Migrations/JellyfinMigrationService.cs | 10 +++++- Jellyfin.Server/Program.cs | 34 ++++++++++++++++--- .../ServerSetupApp/StartupActivity.cs | 3 ++ .../IJellyfinDatabaseProvider.cs | 3 +- .../SqliteDatabaseProvider.cs | 3 +- 6 files changed, 48 insertions(+), 9 deletions(-) diff --git a/Emby.Server.Implementations/ScheduledTasks/Tasks/OptimizeDatabaseTask.cs b/Emby.Server.Implementations/ScheduledTasks/Tasks/OptimizeDatabaseTask.cs index 8d133dc074..687947616f 100644 --- a/Emby.Server.Implementations/ScheduledTasks/Tasks/OptimizeDatabaseTask.cs +++ b/Emby.Server.Implementations/ScheduledTasks/Tasks/OptimizeDatabaseTask.cs @@ -11,7 +11,7 @@ using Microsoft.Extensions.Logging; namespace Emby.Server.Implementations.ScheduledTasks.Tasks; /// -/// Optimizes Jellyfin's database by issuing a VACUUM command. +/// Optimizes Jellyfin's database by issuing VACUUM and ANALYZE commands. /// public class OptimizeDatabaseTask : IScheduledTask, IConfigurableScheduledTask { @@ -82,7 +82,7 @@ public class OptimizeDatabaseTask : IScheduledTask, IConfigurableScheduledTask return; } - _logger.LogInformation("Optimizing and vacuuming jellyfin.db..."); + _logger.LogInformation("Vacuuming and analyzing jellyfin.db..."); try { diff --git a/Jellyfin.Server/Migrations/JellyfinMigrationService.cs b/Jellyfin.Server/Migrations/JellyfinMigrationService.cs index beafc3916f..5ef039a843 100644 --- a/Jellyfin.Server/Migrations/JellyfinMigrationService.cs +++ b/Jellyfin.Server/Migrations/JellyfinMigrationService.cs @@ -183,7 +183,13 @@ internal class JellyfinMigrationService } } - public async Task MigrateStepAsync(JellyfinMigrationStageTypes stage, IServiceProvider? serviceProvider) + /// + /// Runs all pending migrations of the requested stage. + /// + /// The stage to migrate. + /// The service provider handed to the migrations. + /// A value indicating whether at least one migration has been applied. + public async Task MigrateStepAsync(JellyfinMigrationStageTypes stage, IServiceProvider? serviceProvider) { var logger = _startupLogger.With(_loggerFactory.CreateLogger()).BeginGroup($"Migrate stage {stage}."); ICollection migrationStage = (Migrations.FirstOrDefault(e => e.Stage == stage) as ICollection) ?? []; @@ -297,6 +303,8 @@ internal class JellyfinMigrationService completedMigrations++; } + + return completedMigrations > 0; } } diff --git a/Jellyfin.Server/Program.cs b/Jellyfin.Server/Program.cs index 12f92efb35..2341af47c1 100644 --- a/Jellyfin.Server/Program.cs +++ b/Jellyfin.Server/Program.cs @@ -61,6 +61,7 @@ namespace Jellyfin.Server private static ILogger _logger = NullLogger.Instance; private static bool _restartOnShutdown; private static IStartupLogger? _migrationLogger; + private static bool _optimizeDatabaseAfterMigration; private static string? _restoreFromBackup; /// @@ -209,14 +210,15 @@ namespace Jellyfin.Server await jellyfinMigrationService.PrepareSystemForMigration(_logger).ConfigureAwait(false); // "Preparing migrations" carries through the DB read; per-migration progress is reported // as "Running migration X of Y" from inside the step once the pending set is known. - await jellyfinMigrationService.MigrateStepAsync(JellyfinMigrationStageTypes.CoreInitialisation, appHost.ServiceProvider).ConfigureAwait(false); + _optimizeDatabaseAfterMigration |= await jellyfinMigrationService.MigrateStepAsync(JellyfinMigrationStageTypes.CoreInitialisation, appHost.ServiceProvider).ConfigureAwait(false); SetupServer.ReportActivity(StartupActivity.InitializingServices); await appHost.InitializeServices(startupConfig).ConfigureAwait(false); _appHost = appHost; - await jellyfinMigrationService.MigrateStepAsync(JellyfinMigrationStageTypes.AppInitialisation, appHost.ServiceProvider).ConfigureAwait(false); + _optimizeDatabaseAfterMigration |= await jellyfinMigrationService.MigrateStepAsync(JellyfinMigrationStageTypes.AppInitialisation, appHost.ServiceProvider).ConfigureAwait(false); await jellyfinMigrationService.CleanupSystemAfterMigration(_logger).ConfigureAwait(false); + await OptimizeDatabaseAfterMigrationAsync(appHost.ServiceProvider).ConfigureAwait(false); try { configurationCompleted = true; @@ -314,7 +316,7 @@ namespace Jellyfin.Server var jellyfinMigrationService = ActivatorUtilities.CreateInstance(startupService); await jellyfinMigrationService.CheckFirstTimeRunOrMigration(appPaths, startupOptions).ConfigureAwait(false); - await jellyfinMigrationService.MigrateStepAsync(Migrations.Stages.JellyfinMigrationStageTypes.PreInitialisation, startupService).ConfigureAwait(false); + _optimizeDatabaseAfterMigration |= await jellyfinMigrationService.MigrateStepAsync(Migrations.Stages.JellyfinMigrationStageTypes.PreInitialisation, startupService).ConfigureAwait(false); } /// @@ -329,7 +331,31 @@ namespace Jellyfin.Server public static async Task ApplyCoreMigrationsAsync(IServiceProvider serviceProvider, Migrations.Stages.JellyfinMigrationStageTypes jellyfinMigrationStage) { var jellyfinMigrationService = ActivatorUtilities.CreateInstance(serviceProvider, _migrationLogger!); - await jellyfinMigrationService.MigrateStepAsync(jellyfinMigrationStage, serviceProvider).ConfigureAwait(false); + _optimizeDatabaseAfterMigration |= await jellyfinMigrationService.MigrateStepAsync(jellyfinMigrationStage, serviceProvider).ConfigureAwait(false); + } + + private static async Task OptimizeDatabaseAfterMigrationAsync(IServiceProvider serviceProvider) + { + if (!_optimizeDatabaseAfterMigration) + { + return; + } + + // Reset first: a restart runs no migrations and must not optimize again. + _optimizeDatabaseAfterMigration = false; + SetupServer.ReportActivity(StartupActivity.OptimizingDatabase); + _logger.LogInformation("Migrations have been applied, optimizing the database... This might take a while"); + + try + { + var databaseProvider = serviceProvider.GetRequiredService(); + await databaseProvider.RunScheduledOptimisation(CancellationToken.None).ConfigureAwait(false); + } + catch (Exception ex) + { + // A missed optimization only costs performance, so never fail startup over this. + _logger.LogError(ex, "Error while optimizing the database after migration"); + } } /// diff --git a/Jellyfin.Server/ServerSetupApp/StartupActivity.cs b/Jellyfin.Server/ServerSetupApp/StartupActivity.cs index 888cc617d4..abfc5bc8ba 100644 --- a/Jellyfin.Server/ServerSetupApp/StartupActivity.cs +++ b/Jellyfin.Server/ServerSetupApp/StartupActivity.cs @@ -27,6 +27,9 @@ public static class StartupActivity /// Bringing up core services and plugins. public const string InitializingServices = "Initializing services"; + /// Refreshing the database statistics after migrations have run. + public const string OptimizingDatabase = "Optimizing database"; + /// Running the final startup tasks. public const string FinishingStartup = "Finishing startup"; diff --git a/src/Jellyfin.Database/Jellyfin.Database.Implementations/IJellyfinDatabaseProvider.cs b/src/Jellyfin.Database/Jellyfin.Database.Implementations/IJellyfinDatabaseProvider.cs index 27dbeaba6a..87d87e92b8 100644 --- a/src/Jellyfin.Database/Jellyfin.Database.Implementations/IJellyfinDatabaseProvider.cs +++ b/src/Jellyfin.Database/Jellyfin.Database.Implementations/IJellyfinDatabaseProvider.cs @@ -37,7 +37,8 @@ public interface IJellyfinDatabaseProvider void ConfigureConventions(ModelConfigurationBuilder configurationBuilder); /// - /// If supported this should run any periodic maintaince tasks. + /// If supported this should run any periodic maintaince tasks, reclaiming unused space and refreshing the query + /// planner statistics. Also used after migrations have modified the database. /// /// The token to abort the operation. /// A representing the asynchronous operation. diff --git a/src/Jellyfin.Database/Jellyfin.Database.Providers.Sqlite/SqliteDatabaseProvider.cs b/src/Jellyfin.Database/Jellyfin.Database.Providers.Sqlite/SqliteDatabaseProvider.cs index 8020fe1f93..dff834bfec 100644 --- a/src/Jellyfin.Database/Jellyfin.Database.Providers.Sqlite/SqliteDatabaseProvider.cs +++ b/src/Jellyfin.Database/Jellyfin.Database.Providers.Sqlite/SqliteDatabaseProvider.cs @@ -109,8 +109,9 @@ public sealed class SqliteDatabaseProvider : IJellyfinDatabaseProvider await using (context.ConfigureAwait(false)) { await context.Database.ExecuteSqlRawAsync("PRAGMA wal_checkpoint(TRUNCATE)", cancellationToken).ConfigureAwait(false); - await context.Database.ExecuteSqlRawAsync("PRAGMA optimize", cancellationToken).ConfigureAwait(false); await context.Database.ExecuteSqlRawAsync("VACUUM", cancellationToken).ConfigureAwait(false); + await context.Database.ExecuteSqlRawAsync("PRAGMA analysis_limit=0", cancellationToken).ConfigureAwait(false); + await context.Database.ExecuteSqlRawAsync("ANALYZE", cancellationToken).ConfigureAwait(false); await context.Database.ExecuteSqlRawAsync("PRAGMA wal_checkpoint(TRUNCATE)", cancellationToken).ConfigureAwait(false); _logger.LogInformation("jellyfin.db optimized successfully!"); } From 5465e0c69423506d6836f2a6b8aacfce8cb0501e Mon Sep 17 00:00:00 2001 From: Shadowghost Date: Sat, 5 Sep 2026 07:34:38 +0200 Subject: [PATCH 09/17] Optimize the database on startup instead of before shutdown --- .../Migrations/JellyfinMigrationService.cs | 6 ++-- Jellyfin.Server/Program.cs | 30 +++++++------------ .../ServerSetupApp/StartupActivity.cs | 2 +- .../IJellyfinDatabaseProvider.cs | 2 +- .../SqliteDatabaseProvider.cs | 14 ++------- 5 files changed, 16 insertions(+), 38 deletions(-) diff --git a/Jellyfin.Server/Migrations/JellyfinMigrationService.cs b/Jellyfin.Server/Migrations/JellyfinMigrationService.cs index 5ef039a843..6ecbe6a95e 100644 --- a/Jellyfin.Server/Migrations/JellyfinMigrationService.cs +++ b/Jellyfin.Server/Migrations/JellyfinMigrationService.cs @@ -188,8 +188,8 @@ internal class JellyfinMigrationService /// /// The stage to migrate. /// The service provider handed to the migrations. - /// A value indicating whether at least one migration has been applied. - public async Task MigrateStepAsync(JellyfinMigrationStageTypes stage, IServiceProvider? serviceProvider) + /// A representing the asynchronous operation. + public async Task MigrateStepAsync(JellyfinMigrationStageTypes stage, IServiceProvider? serviceProvider) { var logger = _startupLogger.With(_loggerFactory.CreateLogger()).BeginGroup($"Migrate stage {stage}."); ICollection migrationStage = (Migrations.FirstOrDefault(e => e.Stage == stage) as ICollection) ?? []; @@ -303,8 +303,6 @@ internal class JellyfinMigrationService completedMigrations++; } - - return completedMigrations > 0; } } diff --git a/Jellyfin.Server/Program.cs b/Jellyfin.Server/Program.cs index 2341af47c1..2391c86641 100644 --- a/Jellyfin.Server/Program.cs +++ b/Jellyfin.Server/Program.cs @@ -61,7 +61,6 @@ namespace Jellyfin.Server private static ILogger _logger = NullLogger.Instance; private static bool _restartOnShutdown; private static IStartupLogger? _migrationLogger; - private static bool _optimizeDatabaseAfterMigration; private static string? _restoreFromBackup; /// @@ -210,15 +209,15 @@ namespace Jellyfin.Server await jellyfinMigrationService.PrepareSystemForMigration(_logger).ConfigureAwait(false); // "Preparing migrations" carries through the DB read; per-migration progress is reported // as "Running migration X of Y" from inside the step once the pending set is known. - _optimizeDatabaseAfterMigration |= await jellyfinMigrationService.MigrateStepAsync(JellyfinMigrationStageTypes.CoreInitialisation, appHost.ServiceProvider).ConfigureAwait(false); + await jellyfinMigrationService.MigrateStepAsync(JellyfinMigrationStageTypes.CoreInitialisation, appHost.ServiceProvider).ConfigureAwait(false); SetupServer.ReportActivity(StartupActivity.InitializingServices); await appHost.InitializeServices(startupConfig).ConfigureAwait(false); _appHost = appHost; - _optimizeDatabaseAfterMigration |= await jellyfinMigrationService.MigrateStepAsync(JellyfinMigrationStageTypes.AppInitialisation, appHost.ServiceProvider).ConfigureAwait(false); + await jellyfinMigrationService.MigrateStepAsync(JellyfinMigrationStageTypes.AppInitialisation, appHost.ServiceProvider).ConfigureAwait(false); await jellyfinMigrationService.CleanupSystemAfterMigration(_logger).ConfigureAwait(false); - await OptimizeDatabaseAfterMigrationAsync(appHost.ServiceProvider).ConfigureAwait(false); + await OptimizeDatabaseAsync(appHost.ServiceProvider).ConfigureAwait(false); try { configurationCompleted = true; @@ -273,11 +272,8 @@ namespace Jellyfin.Server // Don't throw additional exception if startup failed. if (appHost.ServiceProvider is not null) { - _logger.LogInformation("Running query planner optimizations in the database... This might take a while"); - var databaseProvider = appHost.ServiceProvider.GetRequiredService(); - using var shutdownSource = new CancellationTokenSource(); - shutdownSource.CancelAfter((int)TimeSpan.FromSeconds(60).TotalMicroseconds); + using var shutdownSource = new CancellationTokenSource(TimeSpan.FromSeconds(60)); await databaseProvider.RunShutdownTask(shutdownSource.Token).ConfigureAwait(false); } @@ -316,7 +312,7 @@ namespace Jellyfin.Server var jellyfinMigrationService = ActivatorUtilities.CreateInstance(startupService); await jellyfinMigrationService.CheckFirstTimeRunOrMigration(appPaths, startupOptions).ConfigureAwait(false); - _optimizeDatabaseAfterMigration |= await jellyfinMigrationService.MigrateStepAsync(Migrations.Stages.JellyfinMigrationStageTypes.PreInitialisation, startupService).ConfigureAwait(false); + await jellyfinMigrationService.MigrateStepAsync(Migrations.Stages.JellyfinMigrationStageTypes.PreInitialisation, startupService).ConfigureAwait(false); } /// @@ -331,30 +327,24 @@ namespace Jellyfin.Server public static async Task ApplyCoreMigrationsAsync(IServiceProvider serviceProvider, Migrations.Stages.JellyfinMigrationStageTypes jellyfinMigrationStage) { var jellyfinMigrationService = ActivatorUtilities.CreateInstance(serviceProvider, _migrationLogger!); - _optimizeDatabaseAfterMigration |= await jellyfinMigrationService.MigrateStepAsync(jellyfinMigrationStage, serviceProvider).ConfigureAwait(false); + await jellyfinMigrationService.MigrateStepAsync(jellyfinMigrationStage, serviceProvider).ConfigureAwait(false); } - private static async Task OptimizeDatabaseAfterMigrationAsync(IServiceProvider serviceProvider) + private static async Task OptimizeDatabaseAsync(IServiceProvider serviceProvider) { - if (!_optimizeDatabaseAfterMigration) - { - return; - } - - // Reset first: a restart runs no migrations and must not optimize again. - _optimizeDatabaseAfterMigration = false; SetupServer.ReportActivity(StartupActivity.OptimizingDatabase); - _logger.LogInformation("Migrations have been applied, optimizing the database... This might take a while"); + _logger.LogInformation("Vacuuming and analyzing the database... This might take a while"); try { + // Deliberately untimed: incomplete statistics are worse than a slow start. var databaseProvider = serviceProvider.GetRequiredService(); await databaseProvider.RunScheduledOptimisation(CancellationToken.None).ConfigureAwait(false); } catch (Exception ex) { // A missed optimization only costs performance, so never fail startup over this. - _logger.LogError(ex, "Error while optimizing the database after migration"); + _logger.LogError(ex, "Error while optimizing the database"); } } diff --git a/Jellyfin.Server/ServerSetupApp/StartupActivity.cs b/Jellyfin.Server/ServerSetupApp/StartupActivity.cs index abfc5bc8ba..5fc485bd48 100644 --- a/Jellyfin.Server/ServerSetupApp/StartupActivity.cs +++ b/Jellyfin.Server/ServerSetupApp/StartupActivity.cs @@ -27,7 +27,7 @@ public static class StartupActivity /// Bringing up core services and plugins. public const string InitializingServices = "Initializing services"; - /// Refreshing the database statistics after migrations have run. + /// Refreshing the database query planner statistics. public const string OptimizingDatabase = "Optimizing database"; /// Running the final startup tasks. diff --git a/src/Jellyfin.Database/Jellyfin.Database.Implementations/IJellyfinDatabaseProvider.cs b/src/Jellyfin.Database/Jellyfin.Database.Implementations/IJellyfinDatabaseProvider.cs index 87d87e92b8..a7b2aa493b 100644 --- a/src/Jellyfin.Database/Jellyfin.Database.Implementations/IJellyfinDatabaseProvider.cs +++ b/src/Jellyfin.Database/Jellyfin.Database.Implementations/IJellyfinDatabaseProvider.cs @@ -38,7 +38,7 @@ public interface IJellyfinDatabaseProvider /// /// If supported this should run any periodic maintaince tasks, reclaiming unused space and refreshing the query - /// planner statistics. Also used after migrations have modified the database. + /// planner statistics. Also runs on startup once all migrations have been applied. /// /// The token to abort the operation. /// A representing the asynchronous operation. diff --git a/src/Jellyfin.Database/Jellyfin.Database.Providers.Sqlite/SqliteDatabaseProvider.cs b/src/Jellyfin.Database/Jellyfin.Database.Providers.Sqlite/SqliteDatabaseProvider.cs index dff834bfec..6103ecd992 100644 --- a/src/Jellyfin.Database/Jellyfin.Database.Providers.Sqlite/SqliteDatabaseProvider.cs +++ b/src/Jellyfin.Database/Jellyfin.Database.Providers.Sqlite/SqliteDatabaseProvider.cs @@ -124,21 +124,11 @@ public sealed class SqliteDatabaseProvider : IJellyfinDatabaseProvider } /// - public async Task RunShutdownTask(CancellationToken cancellationToken) + public Task RunShutdownTask(CancellationToken cancellationToken) { - if (DbContextFactory is null) - { - return; - } - // Run before disposing the application - var context = await DbContextFactory.CreateDbContextAsync(cancellationToken).ConfigureAwait(false); - await using (context.ConfigureAwait(false)) - { - await context.Database.ExecuteSqlRawAsync("PRAGMA optimize", cancellationToken).ConfigureAwait(false); - } - SqliteConnection.ClearAllPools(); + return Task.CompletedTask; } /// From 5acb200c02d1a4f884dd6f609c027d9187c17f29 Mon Sep 17 00:00:00 2001 From: fmarcac <188743521+fmarcac@users.noreply.github.com> Date: Sat, 5 Sep 2026 14:23:37 +0200 Subject: [PATCH 10/17] Drop SyncPlay requests from sessions that left the group --- Emby.Server.Implementations/SyncPlay/SyncPlayManager.cs | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-) diff --git a/Emby.Server.Implementations/SyncPlay/SyncPlayManager.cs b/Emby.Server.Implementations/SyncPlay/SyncPlayManager.cs index b45d754554..9b12c68ec1 100644 --- a/Emby.Server.Implementations/SyncPlay/SyncPlayManager.cs +++ b/Emby.Server.Implementations/SyncPlay/SyncPlayManager.cs @@ -332,8 +332,11 @@ namespace Emby.Server.Implementations.SyncPlay // Group lock required as Group is not thread-safe. lock (group) { - // Make sure that session still belongs to this group. - if (_sessionToGroupMap.TryGetValue(session.Id, out var checkGroup) && !checkGroup.GroupId.Equals(group.GroupId)) + // Make sure that session still belongs to this group. The lookup can fail + // outright when the session left while this request was waiting on the group + // lock, which is exactly the case this re-check exists to catch. + if (!_sessionToGroupMap.TryGetValue(session.Id, out var checkGroup) + || !checkGroup.GroupId.Equals(group.GroupId)) { // Drop request. return; From 51a7d5d08a3dc00ec3c83f4491be0436a4098096 Mon Sep 17 00:00:00 2001 From: fmarcac <188743521+fmarcac@users.noreply.github.com> Date: Sat, 5 Sep 2026 14:24:48 +0200 Subject: [PATCH 11/17] Fix SyncPlay active session counter leaking on rejoin --- .../SyncPlay/SyncPlayManager.cs | 6 +- .../SyncPlay/SyncPlayManagerTests.cs | 95 +++++++++++++++++++ 2 files changed, 98 insertions(+), 3 deletions(-) create mode 100644 tests/Jellyfin.Server.Implementations.Tests/SyncPlay/SyncPlayManagerTests.cs diff --git a/Emby.Server.Implementations/SyncPlay/SyncPlayManager.cs b/Emby.Server.Implementations/SyncPlay/SyncPlayManager.cs index b45d754554..a2b9088708 100644 --- a/Emby.Server.Implementations/SyncPlay/SyncPlayManager.cs +++ b/Emby.Server.Implementations/SyncPlay/SyncPlayManager.cs @@ -181,8 +181,8 @@ namespace Emby.Server.Implementations.SyncPlay { if (existingGroup.GroupId.Equals(request.GroupId)) { - // Restore session. - UpdateSessionsCounter(session.UserId, 1); + // Restore session. The session is already in the group and has already + // been counted, so the counter must not be incremented a second time. group.SessionJoin(session, request, cancellationToken); return; } @@ -400,7 +400,7 @@ namespace Emby.Server.Implementations.SyncPlay // Update sessions counter. var newSessionsCounter = _activeUsers.AddOrUpdate( userId, - 1, + toAdd, (_, sessionsCounter) => sessionsCounter + toAdd); // Should never happen. diff --git a/tests/Jellyfin.Server.Implementations.Tests/SyncPlay/SyncPlayManagerTests.cs b/tests/Jellyfin.Server.Implementations.Tests/SyncPlay/SyncPlayManagerTests.cs new file mode 100644 index 0000000000..b1221f6f71 --- /dev/null +++ b/tests/Jellyfin.Server.Implementations.Tests/SyncPlay/SyncPlayManagerTests.cs @@ -0,0 +1,95 @@ +using System; +using System.Threading; +using Jellyfin.Database.Implementations.Entities; +using MediaBrowser.Controller.Library; +using MediaBrowser.Controller.Session; +using MediaBrowser.Controller.SyncPlay.Requests; +using Microsoft.Extensions.Logging.Abstractions; +using Moq; +using Xunit; +using SyncPlayManager = Emby.Server.Implementations.SyncPlay.SyncPlayManager; + +namespace Jellyfin.Server.Implementations.Tests.SyncPlay; + +public class SyncPlayManagerTests +{ + [Fact] + public void LeaveGroup_AfterJoiningTheSameGroupTwice_ClearsTheActiveSessionCounter() + { + var harness = new ManagerHarness(); + + var info = harness.Manager.NewGroup(harness.Session, new NewGroupRequest("group"), CancellationToken.None); + Assert.True(harness.Manager.IsUserActive(harness.User.Id)); + + // A client that re-sends Join for the group it is already in must not be counted twice. + harness.Manager.JoinGroup(harness.Session, new JoinGroupRequest(info.GroupId), CancellationToken.None); + harness.Manager.LeaveGroup(harness.Session, new LeaveGroupRequest(), CancellationToken.None); + + Assert.False(harness.Manager.IsUserActive(harness.User.Id)); + } + + [Fact] + public void LeaveGroup_AfterASingleJoin_ClearsTheActiveSessionCounter() + { + var harness = new ManagerHarness(); + + harness.Manager.NewGroup(harness.Session, new NewGroupRequest("group"), CancellationToken.None); + harness.Manager.LeaveGroup(harness.Session, new LeaveGroupRequest(), CancellationToken.None); + + Assert.False(harness.Manager.IsUserActive(harness.User.Id)); + } + + [Fact] + public void IsUserActive_WithTwoSessionsOfTheSameUser_TracksBothSeparately() + { + var harness = new ManagerHarness(); + var second = harness.CreateSession("session-2"); + + var info = harness.Manager.NewGroup(harness.Session, new NewGroupRequest("group"), CancellationToken.None); + harness.Manager.JoinGroup(second, new JoinGroupRequest(info.GroupId), CancellationToken.None); + + harness.Manager.LeaveGroup(harness.Session, new LeaveGroupRequest(), CancellationToken.None); + Assert.True(harness.Manager.IsUserActive(harness.User.Id)); + + harness.Manager.LeaveGroup(second, new LeaveGroupRequest(), CancellationToken.None); + Assert.False(harness.Manager.IsUserActive(harness.User.Id)); + } + + private sealed class ManagerHarness + { + private readonly Mock _sessionManager = new(); + + public ManagerHarness() + { + var userManager = new Mock(); + var libraryManager = new Mock(); + + User = new User("tester", "auth-provider", "pwdreset-provider"); + userManager.Setup(m => m.GetUserById(It.IsAny())).Returns(User); + + Manager = new SyncPlayManager( + NullLoggerFactory.Instance, + userManager.Object, + _sessionManager.Object, + libraryManager.Object); + + Session = CreateSession("session-1"); + } + + public SyncPlayManager Manager { get; } + + public User User { get; } + + public SessionInfo Session { get; } + + public SessionInfo CreateSession(string id) + { + return new SessionInfo(_sessionManager.Object, NullLogger.Instance) + { + Id = id, + UserId = User.Id, + UserName = User.Username + }; + } + } +} From e5bfe562bc04b23afd9dcf1cb7067c94372558be Mon Sep 17 00:00:00 2001 From: fmarcac <188743521+fmarcac@users.noreply.github.com> Date: Sat, 5 Sep 2026 14:19:01 +0200 Subject: [PATCH 12/17] Fix crash when SyncPlay shuffle mode is set to sorted twice --- .../SyncPlay/Queue/PlayQueueManager.cs | 5 +++- .../SyncPlay/PlayQueueManagerTests.cs | 30 +++++++++++++++++++ 2 files changed, 34 insertions(+), 1 deletion(-) diff --git a/MediaBrowser.Controller/SyncPlay/Queue/PlayQueueManager.cs b/MediaBrowser.Controller/SyncPlay/Queue/PlayQueueManager.cs index 9326864d78..258b92e4d9 100644 --- a/MediaBrowser.Controller/SyncPlay/Queue/PlayQueueManager.cs +++ b/MediaBrowser.Controller/SyncPlay/Queue/PlayQueueManager.cs @@ -157,7 +157,10 @@ namespace MediaBrowser.Controller.SyncPlay.Queue /// public void RestoreSortedPlaylist() { - if (PlayingItemIndex != NoPlayingItemIndex) + // The shuffled playlist is only populated while the shuffle mode is active, so there is + // nothing to map back when the playlist is already sorted. Guarding on its contents keeps + // a redundant request for the sorted mode from indexing an empty list. + if (PlayingItemIndex != NoPlayingItemIndex && _shuffledPlaylist.Count > 0) { var playingItem = _shuffledPlaylist[PlayingItemIndex]; PlayingItemIndex = _sortedPlaylist.IndexOf(playingItem); diff --git a/tests/Jellyfin.Server.Implementations.Tests/SyncPlay/PlayQueueManagerTests.cs b/tests/Jellyfin.Server.Implementations.Tests/SyncPlay/PlayQueueManagerTests.cs index 32685556b2..05e8a40de1 100644 --- a/tests/Jellyfin.Server.Implementations.Tests/SyncPlay/PlayQueueManagerTests.cs +++ b/tests/Jellyfin.Server.Implementations.Tests/SyncPlay/PlayQueueManagerTests.cs @@ -142,6 +142,36 @@ public class PlayQueueManagerTests Assert.Equal(Guid.Empty, queue.GetPlayingItemPlaylistId()); } + [Fact] + public void SetShuffleMode_SortedWhileAlreadySorted_KeepsPlayingItem() + { + var queue = CreateQueue(3); + queue.SetPlayingItemByIndex(1); + var expectedItemId = queue.GetPlayingItemId(); + + queue.SetShuffleMode(GroupShuffleMode.Sorted); + + Assert.Equal(GroupShuffleMode.Sorted, queue.ShuffleMode); + Assert.Equal(1, queue.PlayingItemIndex); + Assert.Equal(expectedItemId, queue.GetPlayingItemId()); + } + + [Fact] + public void SetShuffleMode_SortedTwiceAfterShuffle_KeepsPlayingItem() + { + var queue = CreateQueue(5); + queue.SetPlayingItemByIndex(2); + var expectedItemId = queue.GetPlayingItemId(); + + queue.SetShuffleMode(GroupShuffleMode.Shuffle); + queue.SetShuffleMode(GroupShuffleMode.Sorted); + queue.SetShuffleMode(GroupShuffleMode.Sorted); + + Assert.Equal(GroupShuffleMode.Sorted, queue.ShuffleMode); + Assert.Equal(5, queue.GetPlaylist().Count); + Assert.Equal(expectedItemId, queue.GetPlayingItemId()); + } + [Fact] public void SetPlayingItemByIndex_InBounds_SetsPlayingItem() { From 0c05d9d1a97ccf77571c7cffd39314c5cf3ad4e6 Mon Sep 17 00:00:00 2001 From: fmarcac <188743521+fmarcac@users.noreply.github.com> Date: Sat, 5 Sep 2026 14:20:26 +0200 Subject: [PATCH 13/17] Apply the SyncPlay resume delay floor in the correct unit --- .../SyncPlay/GroupStates/WaitingGroupState.cs | 2 +- .../SyncPlay/WaitingGroupStateTests.cs | 109 ++++++++++++++++++ 2 files changed, 110 insertions(+), 1 deletion(-) create mode 100644 tests/Jellyfin.Server.Implementations.Tests/SyncPlay/WaitingGroupStateTests.cs diff --git a/MediaBrowser.Controller/SyncPlay/GroupStates/WaitingGroupState.cs b/MediaBrowser.Controller/SyncPlay/GroupStates/WaitingGroupState.cs index eb38eeb503..f4fab29800 100644 --- a/MediaBrowser.Controller/SyncPlay/GroupStates/WaitingGroupState.cs +++ b/MediaBrowser.Controller/SyncPlay/GroupStates/WaitingGroupState.cs @@ -501,7 +501,7 @@ namespace MediaBrowser.Controller.SyncPlay.GroupStates { // Client, that was buffering, resumed playback but did not update others in time. delayTicks = context.GetHighestPing() * 2 * TimeSpan.TicksPerMillisecond; - delayTicks = Math.Max(delayTicks, context.DefaultPing); + delayTicks = Math.Max(delayTicks, TimeSpan.FromMilliseconds(context.DefaultPing).Ticks); context.LastActivity = currentTime.AddTicks(delayTicks); diff --git a/tests/Jellyfin.Server.Implementations.Tests/SyncPlay/WaitingGroupStateTests.cs b/tests/Jellyfin.Server.Implementations.Tests/SyncPlay/WaitingGroupStateTests.cs new file mode 100644 index 0000000000..1e2467b3fc --- /dev/null +++ b/tests/Jellyfin.Server.Implementations.Tests/SyncPlay/WaitingGroupStateTests.cs @@ -0,0 +1,109 @@ +using System; +using System.Collections.Generic; +using System.Threading; +using System.Threading.Tasks; +using Jellyfin.Database.Implementations.Entities; +using MediaBrowser.Controller.Entities; +using MediaBrowser.Controller.Library; +using MediaBrowser.Controller.Session; +using MediaBrowser.Controller.SyncPlay.GroupStates; +using MediaBrowser.Controller.SyncPlay.PlaybackRequests; +using MediaBrowser.Controller.SyncPlay.Requests; +using MediaBrowser.Model.SyncPlay; +using Microsoft.Extensions.Logging.Abstractions; +using Moq; +using Xunit; +using SyncPlayGroup = Emby.Server.Implementations.SyncPlay.Group; + +namespace Jellyfin.Server.Implementations.Tests.SyncPlay; + +public class WaitingGroupStateTests +{ + [Fact] + public void Ready_ClientResumedWithLowPing_AppliesTheDefaultPingFloorInMilliseconds() + { + var harness = new GroupHarness(); + var group = harness.Group; + + // Both members report a ping well under the default, so the floor is what decides the delay. + group.UpdatePing(harness.First, 10); + group.UpdatePing(harness.Second, 10); + + group.PositionTicks = TimeSpan.FromMinutes(5).Ticks; + group.LastActivity = DateTime.UtcNow; + group.SetBuffering(harness.First, true); + group.SetBuffering(harness.Second, false); + + var state = new WaitingGroupState(NullLoggerFactory.Instance) { ResumePlaying = true }; + + var before = DateTime.UtcNow; + state.HandleRequest( + new ReadyGroupRequest(DateTime.UtcNow, group.PositionTicks, true, harness.PlaylistItemId), + group, + GroupStateType.Waiting, + harness.First, + CancellationToken.None); + + // DefaultPing is expressed in milliseconds, so the floor must be converted before being + // compared against a tick count. Without the conversion the floor is 500 ticks (0.05 ms) + // and never applies. + var scheduledDelay = group.LastActivity - before; + Assert.True( + scheduledDelay >= TimeSpan.FromMilliseconds(group.DefaultPing), + $"expected a resume delay of at least {group.DefaultPing} ms, got {scheduledDelay.TotalMilliseconds} ms"); + } + + private sealed class GroupHarness + { + public GroupHarness() + { + var userManager = new Mock(); + var sessionManager = new Mock(); + var libraryManager = new Mock(); + + var user = new User("tester", "auth-provider", "pwdreset-provider"); + userManager.Setup(m => m.GetUserById(It.IsAny())).Returns(user); + + var item = new Mock(); + item.Setup(i => i.IsVisibleStandalone(It.IsAny())).Returns(true); + item.Object.RunTimeTicks = TimeSpan.FromHours(2).Ticks; + libraryManager.Setup(m => m.GetItemById(It.IsAny())).Returns(item.Object); + + sessionManager + .Setup(m => m.SendSyncPlayCommand(It.IsAny(), It.IsAny(), It.IsAny())) + .Returns(Task.CompletedTask); + + Group = new SyncPlayGroup( + NullLoggerFactory.Instance, + userManager.Object, + sessionManager.Object, + libraryManager.Object); + + First = new SessionInfo(sessionManager.Object, NullLogger.Instance) + { + Id = "first", + UserId = user.Id, + UserName = "first" + }; + Second = new SessionInfo(sessionManager.Object, NullLogger.Instance) + { + Id = "second", + UserId = user.Id, + UserName = "second" + }; + + Group.CreateGroup(First, new NewGroupRequest("group"), CancellationToken.None); + Group.SessionJoin(Second, new JoinGroupRequest(Group.GroupId), CancellationToken.None); + Group.SetPlayQueue(new List { Guid.NewGuid() }, 0, 0); + PlaylistItemId = Group.PlayQueue.GetPlayingItemPlaylistId(); + } + + public SyncPlayGroup Group { get; } + + public SessionInfo First { get; } + + public SessionInfo Second { get; } + + public Guid PlaylistItemId { get; } + } +} From e356fe9146bb4c62226e9c5108bfa9bf17171f68 Mon Sep 17 00:00:00 2001 From: fmarcac <188743521+fmarcac@users.noreply.github.com> Date: Sat, 5 Sep 2026 14:20:26 +0200 Subject: [PATCH 14/17] Return the default ping for an empty SyncPlay group --- Emby.Server.Implementations/SyncPlay/Group.cs | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/Emby.Server.Implementations/SyncPlay/Group.cs b/Emby.Server.Implementations/SyncPlay/Group.cs index 38a0018a70..256faffbf4 100644 --- a/Emby.Server.Implementations/SyncPlay/Group.cs +++ b/Emby.Server.Implementations/SyncPlay/Group.cs @@ -451,7 +451,9 @@ namespace Emby.Server.Implementations.SyncPlay max = Math.Max(max, session.Ping); } - return max; + // A group with no participants has no ping to report. Returning long.MinValue would + // overflow the callers that scale this value into ticks, so fall back to the default. + return max == long.MinValue ? DefaultPing : max; } /// From 7ce911a40145fce2600ba3ce042b82f7cb97d7f7 Mon Sep 17 00:00:00 2001 From: fmarcac <188743521+fmarcac@users.noreply.github.com> Date: Sat, 5 Sep 2026 15:06:37 +0200 Subject: [PATCH 15/17] Clamp client reported ping in SyncPlay groups --- Emby.Server.Implementations/SyncPlay/Group.cs | 14 +++++++- .../SyncPlay/WaitingGroupStateTests.cs | 32 +++++++++++++++++++ 2 files changed, 45 insertions(+), 1 deletion(-) diff --git a/Emby.Server.Implementations/SyncPlay/Group.cs b/Emby.Server.Implementations/SyncPlay/Group.cs index 256faffbf4..923bfc67aa 100644 --- a/Emby.Server.Implementations/SyncPlay/Group.cs +++ b/Emby.Server.Implementations/SyncPlay/Group.cs @@ -90,6 +90,18 @@ namespace Emby.Server.Implementations.SyncPlay /// The default ping. public long DefaultPing { get; } = 500; + /// + /// Gets the maximum ping, in milliseconds, accepted from a session. + /// + /// + /// Pings are reported by clients and are scaled into the delays used to schedule playback, + /// so an unbounded value lets a single session push the whole group's resume point + /// arbitrarily far out, or overflow the arithmetic entirely. Anything above this is not a + /// usable measurement for synchronisation. + /// + /// The maximum ping. + public long MaxPing { get; } = 10000; + /// /// Gets the maximum time offset error accepted for dates reported by clients, in milliseconds. /// @@ -438,7 +450,7 @@ namespace Emby.Server.Implementations.SyncPlay { if (_participants.TryGetValue(session.Id, out GroupMember value)) { - value.Ping = ping; + value.Ping = Math.Clamp(ping, 0, MaxPing); } } diff --git a/tests/Jellyfin.Server.Implementations.Tests/SyncPlay/WaitingGroupStateTests.cs b/tests/Jellyfin.Server.Implementations.Tests/SyncPlay/WaitingGroupStateTests.cs index 1e2467b3fc..0cccd5d4ca 100644 --- a/tests/Jellyfin.Server.Implementations.Tests/SyncPlay/WaitingGroupStateTests.cs +++ b/tests/Jellyfin.Server.Implementations.Tests/SyncPlay/WaitingGroupStateTests.cs @@ -53,6 +53,34 @@ public class WaitingGroupStateTests $"expected a resume delay of at least {group.DefaultPing} ms, got {scheduledDelay.TotalMilliseconds} ms"); } + [Theory] + [InlineData(4_000_000_000L)] + [InlineData(1_000_000_000_000_000L)] + [InlineData(long.MaxValue)] + [InlineData(-1L)] + public void UpdatePing_ClientReportsAnUnusablePing_IsClampedAndCannotStallTheGroup(long reportedPing) + { + var harness = new GroupHarness(); + var group = harness.Group; + + group.UpdatePing(harness.First, reportedPing); + + Assert.InRange(group.GetHighestPing(), 0, group.MaxPing); + + // The reported ping is scaled into the group's resume point, so an unclamped value either + // pushes playback months out or overflows the arithmetic outright. + var state = new PlayingGroupState(NullLoggerFactory.Instance); + var before = DateTime.UtcNow; + state.HandleRequest( + new UnpauseGroupRequest(), + group, + GroupStateType.Paused, + harness.First, + CancellationToken.None); + + Assert.InRange(group.LastActivity - before, TimeSpan.Zero, TimeSpan.FromMinutes(1)); + } + private sealed class GroupHarness { public GroupHarness() @@ -73,6 +101,10 @@ public class WaitingGroupStateTests .Setup(m => m.SendSyncPlayCommand(It.IsAny(), It.IsAny(), It.IsAny())) .Returns(Task.CompletedTask); + sessionManager + .Setup(m => m.SendSyncPlayGroupUpdate(It.IsAny(), It.IsAny>(), It.IsAny())) + .Returns(Task.CompletedTask); + Group = new SyncPlayGroup( NullLoggerFactory.Instance, userManager.Object, From 9528d0e601e87bbff79d46f0c5f30fecee62a526 Mon Sep 17 00:00:00 2001 From: Oggeb1 <51484222+Oggeb1@users.noreply.github.com> Date: Sat, 5 Sep 2026 17:15:00 +0200 Subject: [PATCH 16/17] Fix BDMV subtitles with external audio Co-authored-by: gnattu --- MediaBrowser.Controller/MediaEncoding/EncodingHelper.cs | 1 + 1 file changed, 1 insertion(+) diff --git a/MediaBrowser.Controller/MediaEncoding/EncodingHelper.cs b/MediaBrowser.Controller/MediaEncoding/EncodingHelper.cs index 8cd191c055..f0c2580e08 100644 --- a/MediaBrowser.Controller/MediaEncoding/EncodingHelper.cs +++ b/MediaBrowser.Controller/MediaEncoding/EncodingHelper.cs @@ -7953,6 +7953,7 @@ namespace MediaBrowser.Controller.MediaEncoding var hiddenStreamsBefore = mediaSource.MediaStreams.Count(s => s.Type == MediaStreamType.Audio + && !s.IsExternal && (string.Equals(s.Codec, "truehd", StringComparison.OrdinalIgnoreCase) || string.Equals(s.Codec, "atmos", StringComparison.OrdinalIgnoreCase)) && s.Index < subtitleStream.Index); From 74ce774effac4d70728e231e6de7c1c5d82a2685 Mon Sep 17 00:00:00 2001 From: Shadowghost Date: Sat, 5 Sep 2026 17:33:47 +0200 Subject: [PATCH 17/17] Revert to doing after migration and on shutdown --- .../Migrations/JellyfinMigrationService.cs | 6 ++- Jellyfin.Server/Program.cs | 30 ++++++++----- .../ServerSetupApp/StartupActivity.cs | 2 +- .../IJellyfinDatabaseProvider.cs | 5 ++- .../SqliteDatabaseProvider.cs | 45 +++++++++++++------ 5 files changed, 60 insertions(+), 28 deletions(-) diff --git a/Jellyfin.Server/Migrations/JellyfinMigrationService.cs b/Jellyfin.Server/Migrations/JellyfinMigrationService.cs index 6ecbe6a95e..5ef039a843 100644 --- a/Jellyfin.Server/Migrations/JellyfinMigrationService.cs +++ b/Jellyfin.Server/Migrations/JellyfinMigrationService.cs @@ -188,8 +188,8 @@ internal class JellyfinMigrationService /// /// The stage to migrate. /// The service provider handed to the migrations. - /// A representing the asynchronous operation. - public async Task MigrateStepAsync(JellyfinMigrationStageTypes stage, IServiceProvider? serviceProvider) + /// A value indicating whether at least one migration has been applied. + public async Task MigrateStepAsync(JellyfinMigrationStageTypes stage, IServiceProvider? serviceProvider) { var logger = _startupLogger.With(_loggerFactory.CreateLogger()).BeginGroup($"Migrate stage {stage}."); ICollection migrationStage = (Migrations.FirstOrDefault(e => e.Stage == stage) as ICollection) ?? []; @@ -303,6 +303,8 @@ internal class JellyfinMigrationService completedMigrations++; } + + return completedMigrations > 0; } } diff --git a/Jellyfin.Server/Program.cs b/Jellyfin.Server/Program.cs index 2391c86641..58861fc476 100644 --- a/Jellyfin.Server/Program.cs +++ b/Jellyfin.Server/Program.cs @@ -61,6 +61,7 @@ namespace Jellyfin.Server private static ILogger _logger = NullLogger.Instance; private static bool _restartOnShutdown; private static IStartupLogger? _migrationLogger; + private static bool _optimizeDatabaseAfterMigration; private static string? _restoreFromBackup; /// @@ -209,15 +210,15 @@ namespace Jellyfin.Server await jellyfinMigrationService.PrepareSystemForMigration(_logger).ConfigureAwait(false); // "Preparing migrations" carries through the DB read; per-migration progress is reported // as "Running migration X of Y" from inside the step once the pending set is known. - await jellyfinMigrationService.MigrateStepAsync(JellyfinMigrationStageTypes.CoreInitialisation, appHost.ServiceProvider).ConfigureAwait(false); + _optimizeDatabaseAfterMigration |= await jellyfinMigrationService.MigrateStepAsync(JellyfinMigrationStageTypes.CoreInitialisation, appHost.ServiceProvider).ConfigureAwait(false); SetupServer.ReportActivity(StartupActivity.InitializingServices); await appHost.InitializeServices(startupConfig).ConfigureAwait(false); _appHost = appHost; - await jellyfinMigrationService.MigrateStepAsync(JellyfinMigrationStageTypes.AppInitialisation, appHost.ServiceProvider).ConfigureAwait(false); + _optimizeDatabaseAfterMigration |= await jellyfinMigrationService.MigrateStepAsync(JellyfinMigrationStageTypes.AppInitialisation, appHost.ServiceProvider).ConfigureAwait(false); await jellyfinMigrationService.CleanupSystemAfterMigration(_logger).ConfigureAwait(false); - await OptimizeDatabaseAsync(appHost.ServiceProvider).ConfigureAwait(false); + await OptimizeDatabaseAfterMigrationAsync(appHost.ServiceProvider).ConfigureAwait(false); try { configurationCompleted = true; @@ -272,9 +273,11 @@ namespace Jellyfin.Server // Don't throw additional exception if startup failed. if (appHost.ServiceProvider is not null) { + _logger.LogInformation("Optimizing the database... This might take a while"); + + // Deliberately untimed: a truncated optimization leaves the statistics incomplete. var databaseProvider = appHost.ServiceProvider.GetRequiredService(); - using var shutdownSource = new CancellationTokenSource(TimeSpan.FromSeconds(60)); - await databaseProvider.RunShutdownTask(shutdownSource.Token).ConfigureAwait(false); + await databaseProvider.RunShutdownTask(CancellationToken.None).ConfigureAwait(false); } _appHost = null; @@ -312,7 +315,7 @@ namespace Jellyfin.Server var jellyfinMigrationService = ActivatorUtilities.CreateInstance(startupService); await jellyfinMigrationService.CheckFirstTimeRunOrMigration(appPaths, startupOptions).ConfigureAwait(false); - await jellyfinMigrationService.MigrateStepAsync(Migrations.Stages.JellyfinMigrationStageTypes.PreInitialisation, startupService).ConfigureAwait(false); + _optimizeDatabaseAfterMigration |= await jellyfinMigrationService.MigrateStepAsync(Migrations.Stages.JellyfinMigrationStageTypes.PreInitialisation, startupService).ConfigureAwait(false); } /// @@ -327,13 +330,20 @@ namespace Jellyfin.Server public static async Task ApplyCoreMigrationsAsync(IServiceProvider serviceProvider, Migrations.Stages.JellyfinMigrationStageTypes jellyfinMigrationStage) { var jellyfinMigrationService = ActivatorUtilities.CreateInstance(serviceProvider, _migrationLogger!); - await jellyfinMigrationService.MigrateStepAsync(jellyfinMigrationStage, serviceProvider).ConfigureAwait(false); + _optimizeDatabaseAfterMigration |= await jellyfinMigrationService.MigrateStepAsync(jellyfinMigrationStage, serviceProvider).ConfigureAwait(false); } - private static async Task OptimizeDatabaseAsync(IServiceProvider serviceProvider) + private static async Task OptimizeDatabaseAfterMigrationAsync(IServiceProvider serviceProvider) { + if (!_optimizeDatabaseAfterMigration) + { + return; + } + + // Reset first: a restart runs no migrations and must not optimize again. + _optimizeDatabaseAfterMigration = false; SetupServer.ReportActivity(StartupActivity.OptimizingDatabase); - _logger.LogInformation("Vacuuming and analyzing the database... This might take a while"); + _logger.LogInformation("Migrations have been applied, optimizing the database... This might take a while"); try { @@ -344,7 +354,7 @@ namespace Jellyfin.Server catch (Exception ex) { // A missed optimization only costs performance, so never fail startup over this. - _logger.LogError(ex, "Error while optimizing the database"); + _logger.LogError(ex, "Error while optimizing the database after migration"); } } diff --git a/Jellyfin.Server/ServerSetupApp/StartupActivity.cs b/Jellyfin.Server/ServerSetupApp/StartupActivity.cs index 5fc485bd48..abfc5bc8ba 100644 --- a/Jellyfin.Server/ServerSetupApp/StartupActivity.cs +++ b/Jellyfin.Server/ServerSetupApp/StartupActivity.cs @@ -27,7 +27,7 @@ public static class StartupActivity /// Bringing up core services and plugins. public const string InitializingServices = "Initializing services"; - /// Refreshing the database query planner statistics. + /// Refreshing the database statistics after migrations have run. public const string OptimizingDatabase = "Optimizing database"; /// Running the final startup tasks. diff --git a/src/Jellyfin.Database/Jellyfin.Database.Implementations/IJellyfinDatabaseProvider.cs b/src/Jellyfin.Database/Jellyfin.Database.Implementations/IJellyfinDatabaseProvider.cs index a7b2aa493b..77abb45f2a 100644 --- a/src/Jellyfin.Database/Jellyfin.Database.Implementations/IJellyfinDatabaseProvider.cs +++ b/src/Jellyfin.Database/Jellyfin.Database.Implementations/IJellyfinDatabaseProvider.cs @@ -38,14 +38,15 @@ public interface IJellyfinDatabaseProvider /// /// If supported this should run any periodic maintaince tasks, reclaiming unused space and refreshing the query - /// planner statistics. Also runs on startup once all migrations have been applied. + /// planner statistics. Also used after migrations have modified the database. /// /// The token to abort the operation. /// A representing the asynchronous operation. Task RunScheduledOptimisation(CancellationToken cancellationToken); /// - /// If supported this should perform any actions that are required on stopping the jellyfin server. + /// If supported this should perform any actions that are required on stopping the jellyfin server, including the + /// same maintenance as . /// /// The token that will be used to abort the operation. /// A representing the asynchronous operation. diff --git a/src/Jellyfin.Database/Jellyfin.Database.Providers.Sqlite/SqliteDatabaseProvider.cs b/src/Jellyfin.Database/Jellyfin.Database.Providers.Sqlite/SqliteDatabaseProvider.cs index 6103ecd992..f11cde7e48 100644 --- a/src/Jellyfin.Database/Jellyfin.Database.Providers.Sqlite/SqliteDatabaseProvider.cs +++ b/src/Jellyfin.Database/Jellyfin.Database.Providers.Sqlite/SqliteDatabaseProvider.cs @@ -103,18 +103,9 @@ public sealed class SqliteDatabaseProvider : IJellyfinDatabaseProvider } /// - public async Task RunScheduledOptimisation(CancellationToken cancellationToken) + public Task RunScheduledOptimisation(CancellationToken cancellationToken) { - var context = await DbContextFactory!.CreateDbContextAsync(cancellationToken).ConfigureAwait(false); - await using (context.ConfigureAwait(false)) - { - await context.Database.ExecuteSqlRawAsync("PRAGMA wal_checkpoint(TRUNCATE)", cancellationToken).ConfigureAwait(false); - await context.Database.ExecuteSqlRawAsync("VACUUM", cancellationToken).ConfigureAwait(false); - await context.Database.ExecuteSqlRawAsync("PRAGMA analysis_limit=0", cancellationToken).ConfigureAwait(false); - await context.Database.ExecuteSqlRawAsync("ANALYZE", cancellationToken).ConfigureAwait(false); - await context.Database.ExecuteSqlRawAsync("PRAGMA wal_checkpoint(TRUNCATE)", cancellationToken).ConfigureAwait(false); - _logger.LogInformation("jellyfin.db optimized successfully!"); - } + return OptimizeAsync(cancellationToken); } /// @@ -124,11 +115,39 @@ public sealed class SqliteDatabaseProvider : IJellyfinDatabaseProvider } /// - public Task RunShutdownTask(CancellationToken cancellationToken) + public async Task RunShutdownTask(CancellationToken cancellationToken) { // Run before disposing the application + try + { + await OptimizeAsync(cancellationToken).ConfigureAwait(false); + } + catch (Exception ex) + { + // A missed optimization only costs performance, so never fail the shutdown over this. + _logger.LogError(ex, "Error while optimizing jellyfin.db"); + } + SqliteConnection.ClearAllPools(); - return Task.CompletedTask; + } + + private async Task OptimizeAsync(CancellationToken cancellationToken) + { + if (DbContextFactory is null) + { + return; + } + + var context = await DbContextFactory.CreateDbContextAsync(cancellationToken).ConfigureAwait(false); + await using (context.ConfigureAwait(false)) + { + await context.Database.ExecuteSqlRawAsync("PRAGMA wal_checkpoint(TRUNCATE)", cancellationToken).ConfigureAwait(false); + await context.Database.ExecuteSqlRawAsync("VACUUM", cancellationToken).ConfigureAwait(false); + await context.Database.ExecuteSqlRawAsync("PRAGMA analysis_limit=0", cancellationToken).ConfigureAwait(false); + await context.Database.ExecuteSqlRawAsync("ANALYZE", cancellationToken).ConfigureAwait(false); + await context.Database.ExecuteSqlRawAsync("PRAGMA wal_checkpoint(TRUNCATE)", cancellationToken).ConfigureAwait(false); + _logger.LogInformation("jellyfin.db optimized successfully!"); + } } ///