diff options
6 files changed, 90 insertions, 70 deletions
diff --git a/Emby.Server.Implementations/Library/UserDataManager.cs b/Emby.Server.Implementations/Library/UserDataManager.cs index bedaacee33..61372f8b56 100644 --- a/Emby.Server.Implementations/Library/UserDataManager.cs +++ b/Emby.Server.Implementations/Library/UserDataManager.cs @@ -329,22 +329,21 @@ namespace Emby.Server.Implementations.Library foreach (var (primaryId, versions) in versionGroups) { - Video? resumeVersion = null; UserItemData? resumeData = null; foreach (var version in versions) { + // Consider both in-progress and completed versions so a finished alternate still marks the primary as played. if (userDataByVersion.TryGetValue(version.Id, out var data) - && data.PlaybackPositionTicks > 0 + && (data.PlaybackPositionTicks > 0 || data.Played) && (resumeData is null || (data.LastPlayedDate ?? DateTime.MinValue) > (resumeData.LastPlayedDate ?? DateTime.MinValue))) { - resumeVersion = version; resumeData = data; } } if (resumeData is not null) { - result[primaryId] = new VersionResumeData(resumeData, resumeVersion!.RunTimeTicks); + result[primaryId] = new VersionResumeData(resumeData); } } diff --git a/Jellyfin.Server.Implementations/Item/BaseItemRepository.TranslateQuery.cs b/Jellyfin.Server.Implementations/Item/BaseItemRepository.TranslateQuery.cs index 96d6d2eaff..c234f333af 100644 --- a/Jellyfin.Server.Implementations/Item/BaseItemRepository.TranslateQuery.cs +++ b/Jellyfin.Server.Implementations/Item/BaseItemRepository.TranslateQuery.cs @@ -515,15 +515,15 @@ public sealed partial class BaseItemRepository var hasSeries = filter.IncludeItemTypes.Contains(BaseItemKind.Series); var userId = filter.User!.Id; var isResumable = filter.IsResumable.Value; + var seriesTypeName = _itemTypeLookup.BaseItemKindNames[BaseItemKind.Series]; // In-progress user data rows; alternate versions track their own progress. var inProgress = context.UserData .Where(ud => ud.UserId == userId && ud.PlaybackPositionTicks > 0); + IQueryable<Guid>? resumableSeriesIds = null; if (hasSeries) { - var seriesTypeName = _itemTypeLookup.BaseItemKindNames[BaseItemKind.Series]; - // Aggregate per series in a single GROUP BY pass, instead of three full scans. var seriesEpisodeStats = context.BaseItems .AsNoTracking() @@ -539,39 +539,41 @@ public sealed partial class BaseItemRepository // A series is resumable if it has an in-progress episode, // or if it has both played and unplayed episodes (partially watched). - var resumableSeriesIds = seriesEpisodeStats + resumableSeriesIds = seriesEpisodeStats .Where(s => s.HasInProgress || (s.HasPlayed && s.HasUnplayed)) .Select(s => s.SeriesId); - - // Non-series items: resumable if the item or any of its alternate versions has - // PlaybackPositionTicks > 0. Alternate versions (PrimaryVersionId set) are excluded - // from the base query, so coalesce their progress onto the primary's id. - var resumableMovieIds = inProgress - .Join(context.BaseItems, ud => ud.ItemId, bi => bi.Id, (ud, bi) => bi.PrimaryVersionId ?? bi.Id); - - baseQuery = baseQuery.Where(e => - (e.Type == seriesTypeName && resumableSeriesIds.Contains(e.Id) == isResumable) - || (e.Type != seriesTypeName && resumableMovieIds.Contains(e.Id) == isResumable)); - } - else - { - // Resumable if the item or any of its alternate versions has PlaybackPositionTicks > 0. - // Alternate versions (PrimaryVersionId set) are excluded from the base query, so - // coalesce their progress onto the primary's id. - var resumableMovieIds = inProgress - .Join(context.BaseItems, ud => ud.ItemId, bi => bi.Id, (ud, bi) => bi.PrimaryVersionId ?? bi.Id); - baseQuery = baseQuery.Where(e => resumableMovieIds.Contains(e.Id) == isResumable); } if (isResumable) { - // Multi-version items surface as the version that was actually played. + // Resume queries surface the version that was actually played, which may be an alternate. + // Match each version on its own progress rather than coalescing onto the primary. + var inProgressIds = inProgress.Select(ud => ud.ItemId); + + baseQuery = hasSeries + ? baseQuery.Where(e => + (e.Type == seriesTypeName && resumableSeriesIds!.Contains(e.Id)) + || (e.Type != seriesTypeName && inProgressIds.Contains(e.Id))) + : baseQuery.Where(e => inProgressIds.Contains(e.Id)); + // When several versions of the same item are in progress, keep only the most recently played one. - baseQuery = baseQuery.Where(e => !context.BaseItems + baseQuery = baseQuery.Where(e => e.Type == seriesTypeName || !context.BaseItems .Where(s => s.Id != e.Id && (s.PrimaryVersionId ?? s.Id) == (e.PrimaryVersionId ?? e.Id)) .Any(s => inProgress.Where(su => su.ItemId == s.Id).Max(su => su.LastPlayedDate) > inProgress.Where(eu => eu.ItemId == e.Id).Max(eu => eu.LastPlayedDate))); } + else + { + // Not-resumable queries operate on primaries only. + var resumableMovieIds = inProgress + .Join(context.BaseItems, ud => ud.ItemId, bi => bi.Id, (ud, bi) => bi.PrimaryVersionId ?? bi.Id); + + baseQuery = hasSeries + ? baseQuery.Where(e => + (e.Type == seriesTypeName && !resumableSeriesIds!.Contains(e.Id)) + || (e.Type != seriesTypeName && !resumableMovieIds.Contains(e.Id))) + : baseQuery.Where(e => !resumableMovieIds.Contains(e.Id)); + } } if (filter.ArtistIds.Length > 0) @@ -758,10 +760,13 @@ public sealed partial class BaseItemRepository } else if (filter.OwnerIds.Length == 0 && filter.ExtraTypes.Length == 0 && !filter.IncludeOwnedItems) { - // Exclude alternate versions and owned non-extra items from general queries. - // Alternate versions have PrimaryVersionId set (pointing to their primary). + // Exclude owned non-extra items from general queries. // Extras (trailers, etc.) have OwnerId set but also have ExtraType set - keep those. - baseQuery = baseQuery.Where(e => e.PrimaryVersionId == null && (e.OwnerId == null || e.ExtraType != null)); + // Alternate versions (PrimaryVersionId set) are normally excluded too, but resume queries + // keep them so the actually-played version can surface instead of collapsing onto the primary. + baseQuery = filter.IsResumable == true + ? baseQuery.Where(e => e.OwnerId == null || e.ExtraType != null) + : baseQuery.Where(e => e.PrimaryVersionId == null && (e.OwnerId == null || e.ExtraType != null)); } if (filter.OwnerIds.Length > 0) diff --git a/Jellyfin.Server.Implementations/Item/BaseItemRepository.cs b/Jellyfin.Server.Implementations/Item/BaseItemRepository.cs index 94dedaeba8..57041276b7 100644 --- a/Jellyfin.Server.Implementations/Item/BaseItemRepository.cs +++ b/Jellyfin.Server.Implementations/Item/BaseItemRepository.cs @@ -167,6 +167,14 @@ public sealed partial class BaseItemRepository return false; } + // Resume queries surface the actually-played version (which may be an alternate sharing the + // primary's presentation key). The resumable filter already keeps one version per group, so + // presentation-key grouping must not collapse the surfaced version back onto the primary. + if (query.IsResumable == true) + { + return false; + } + if (query.GroupBySeriesPresentationUniqueKey) { return false; diff --git a/MediaBrowser.Controller/Library/VersionResumeData.cs b/MediaBrowser.Controller/Library/VersionResumeData.cs index ca8a499d97..455fe739ce 100644 --- a/MediaBrowser.Controller/Library/VersionResumeData.cs +++ b/MediaBrowser.Controller/Library/VersionResumeData.cs @@ -1,29 +1,29 @@ +using System; using MediaBrowser.Controller.Entities; using MediaBrowser.Model.Dto; namespace MediaBrowser.Controller.Library { /// <summary> - /// The user data of the alternate version that should drive resume for a multi-version item. + /// The user data of the most recently played alternate version that should drive the completion state of a multi-version item. /// </summary> /// <param name="UserData">The resume version's user data.</param> - /// <param name="RunTimeTicks">The resume version's runtime, used for the progress percentage.</param> - public record VersionResumeData(UserItemData UserData, long? RunTimeTicks) + public record VersionResumeData(UserItemData UserData) { /// <summary> - /// Applies the resume version's playback state to the supplied user data dto, so that an item - /// whose most recent progress lives on an alternate version still reports that progress. + /// Merges the most recently played version's completion state into the supplied user data dto. + /// Only completion (played) propagates to the primary; the in-progress resume position stays on + /// the version that owns it, which is surfaced directly (e.g. in resume queries) so that playback + /// always targets the correct version rather than resuming the primary at another version's offset. /// </summary> /// <param name="dto">The user data dto to update.</param> public void ApplyTo(UserItemDataDto dto) { - dto.PlaybackPositionTicks = UserData.PlaybackPositionTicks; - dto.Played = UserData.Played; - dto.LastPlayedDate = UserData.LastPlayedDate; + dto.Played = dto.Played || UserData.Played; - if (RunTimeTicks > 0 && UserData.PlaybackPositionTicks > 0) + if ((UserData.LastPlayedDate ?? DateTime.MinValue) > (dto.LastPlayedDate ?? DateTime.MinValue)) { - dto.PlayedPercentage = 100.0 * UserData.PlaybackPositionTicks / RunTimeTicks.Value; + dto.LastPlayedDate = UserData.LastPlayedDate; } } } diff --git a/tests/Jellyfin.Controller.Tests/Library/VersionResumeDataTests.cs b/tests/Jellyfin.Controller.Tests/Library/VersionResumeDataTests.cs index 5b0f003019..8642ab07f8 100644 --- a/tests/Jellyfin.Controller.Tests/Library/VersionResumeDataTests.cs +++ b/tests/Jellyfin.Controller.Tests/Library/VersionResumeDataTests.cs @@ -9,36 +9,40 @@ namespace Jellyfin.Controller.Tests.Library; public class VersionResumeDataTests { [Fact] - public void ApplyTo_OverridesResumeFieldsAndPercentage() + public void ApplyTo_PropagatesCompletionButNotPosition() { var lastPlayed = new DateTime(2026, 1, 2, 0, 0, 0, DateTimeKind.Utc); var resume = new VersionResumeData( - new UserItemData { Key = "version", PlaybackPositionTicks = 25, Played = true, LastPlayedDate = lastPlayed }, - RunTimeTicks: 100); + new UserItemData { Key = "version", PlaybackPositionTicks = 25, Played = true, LastPlayedDate = lastPlayed }); var dto = new UserItemDataDto { Key = "primary", PlaybackPositionTicks = 1, Played = false, PlayedPercentage = 1 }; resume.ApplyTo(dto); - Assert.Equal(25, dto.PlaybackPositionTicks); + // Completion state propagates to the primary... Assert.True(dto.Played); Assert.Equal(lastPlayed, dto.LastPlayedDate); - // The percentage is based on the resume version's own runtime, not the primary's. - Assert.NotNull(dto.PlayedPercentage); - Assert.Equal(25.0, dto.PlayedPercentage.Value, 5); + // ...but the in-progress resume position stays on the version that owns it. + Assert.Equal(1, dto.PlaybackPositionTicks); + Assert.Equal(1.0, dto.PlayedPercentage); } [Fact] - public void ApplyTo_WithoutRuntime_LeavesPercentageUntouched() + public void ApplyTo_DoesNotUnsetExistingPlayedOrRegressLastPlayed() { - var resume = new VersionResumeData(new UserItemData { Key = "version", PlaybackPositionTicks = 25 }, null); - var dto = new UserItemDataDto { Key = "primary", PlayedPercentage = 42 }; + var primaryLastPlayed = new DateTime(2026, 1, 5, 0, 0, 0, DateTimeKind.Utc); + var versionLastPlayed = new DateTime(2026, 1, 2, 0, 0, 0, DateTimeKind.Utc); + var resume = new VersionResumeData( + new UserItemData { Key = "version", Played = false, LastPlayedDate = versionLastPlayed }); + + var dto = new UserItemDataDto { Key = "primary", Played = true, LastPlayedDate = primaryLastPlayed }; resume.ApplyTo(dto); - Assert.Equal(25, dto.PlaybackPositionTicks); - Assert.NotNull(dto.PlayedPercentage); - Assert.Equal(42.0, dto.PlayedPercentage.Value, 5); + // A not-yet-completed version must not clear the primary's own completion, and the more recent + // LastPlayedDate is kept. + Assert.True(dto.Played); + Assert.Equal(primaryLastPlayed, dto.LastPlayedDate); } } diff --git a/tests/Jellyfin.Server.Implementations.Tests/Item/AlternateVersionQueryTranslationTests.cs b/tests/Jellyfin.Server.Implementations.Tests/Item/AlternateVersionQueryTranslationTests.cs index 823ff566a7..1f0de153a0 100644 --- a/tests/Jellyfin.Server.Implementations.Tests/Item/AlternateVersionQueryTranslationTests.cs +++ b/tests/Jellyfin.Server.Implementations.Tests/Item/AlternateVersionQueryTranslationTests.cs @@ -37,39 +37,43 @@ public sealed class AlternateVersionQueryTranslationTests : IDisposable } [Fact] - public void ResumeFilter_VersionProgress_SurfacesPrimary() + public void ResumeFilter_VersionProgress_SurfacesPlayedVersion() { - Guid userId, primaryId, otherId; + Guid userId, primaryId, versionId, otherId; using (var ctx = CreateDbContext()) { - (userId, primaryId, otherId) = Seed(ctx); + (userId, primaryId, versionId, otherId) = Seed(ctx); } using (var ctx = CreateDbContext()) { - // Mirrors the resumable filter in BaseItemRepository.TranslateQuery: progress on any - // version coalesces onto the primary's id. var inProgress = ctx.UserData .Where(ud => ud.UserId == userId && ud.PlaybackPositionTicks > 0); - var resumableMovieIds = inProgress - .Join(ctx.BaseItems, ud => ud.ItemId, bi => bi.Id, (ud, bi) => bi.PrimaryVersionId ?? bi.Id); // Scope to the seeded items; EnsureCreated also seeds a placeholder row. - var seededIds = new[] { primaryId, otherId }; + var seededIds = new[] { primaryId, versionId, otherId }; + // Mirrors the resumable=true filter in BaseItemRepository.TranslateQuery. + var inProgressIds = inProgress.Select(ud => ud.ItemId); var resumable = ctx.BaseItems - .Where(e => seededIds.Contains(e.Id) && e.PrimaryVersionId == null) - .Where(e => resumableMovieIds.Contains(e.Id)) + .Where(e => seededIds.Contains(e.Id)) + .Where(e => inProgressIds.Contains(e.Id)) + .Where(e => !ctx.BaseItems + .Where(s => s.Id != e.Id && (s.PrimaryVersionId ?? s.Id) == (e.PrimaryVersionId ?? e.Id)) + .Any(s => inProgress.Where(su => su.ItemId == s.Id).Max(su => su.LastPlayedDate) + > inProgress.Where(eu => eu.ItemId == e.Id).Max(eu => eu.LastPlayedDate))) .Select(e => e.Id) .ToList(); - Assert.Equal([primaryId], resumable); + Assert.Equal([versionId], resumable); - // The inverse (not-resumable) direction must exclude the primary as well. + // The not-resumable direction keeps primaries only. + var resumableMovieIds = inProgress + .Join(ctx.BaseItems, ud => ud.ItemId, bi => bi.Id, (ud, bi) => bi.PrimaryVersionId ?? bi.Id); var notResumable = ctx.BaseItems .Where(e => seededIds.Contains(e.Id) && e.PrimaryVersionId == null) - .Where(e => resumableMovieIds.Contains(e.Id) == false) + .Where(e => !resumableMovieIds.Contains(e.Id)) .Select(e => e.Id) .ToList(); @@ -84,7 +88,7 @@ public sealed class AlternateVersionQueryTranslationTests : IDisposable using (var ctx = CreateDbContext()) { - (userId, primaryId, otherId) = Seed(ctx); + (userId, primaryId, _, otherId) = Seed(ctx); } using (var ctx = CreateDbContext()) @@ -106,7 +110,7 @@ public sealed class AlternateVersionQueryTranslationTests : IDisposable } } - private static (Guid UserId, Guid PrimaryId, Guid OtherId) Seed(JellyfinDbContext ctx) + private static (Guid UserId, Guid PrimaryId, Guid VersionId, Guid OtherId) Seed(JellyfinDbContext ctx) { var user = new User("test", "auth-provider", "reset-provider"); ctx.Users.Add(user); @@ -129,7 +133,7 @@ public sealed class AlternateVersionQueryTranslationTests : IDisposable }); ctx.SaveChanges(); - return (user.Id, primary.Id, other.Id); + return (user.Id, primary.Id, version.Id, other.Id); } private JellyfinDbContext CreateDbContext() |