alteropen/jellyfin
1
0
Fork 0
mirror of https://github.com/jellyfin/jellyfin.git synced 2026-07-05 22:02:53 +01:00
Code Issues Actions 8 Packages Projects Releases Wiki Activity

Fix review comments

Browse Source
This commit is contained in:
Shadowghost
2026-07-02 08:49:11 +02:00
parent 0fb042b740
commit 38f1d9749e
12 changed files with 259 additions and 60 deletions
Download Patch File Download Diff File Expand all files Collapse all files

5
Emby.Server.Implementations/Dto/DtoService.cs
View File

@@ -1272,8 +1272,9 @@ namespace Emby.Server.Implementations.Dto
// Match the per-user filtering of the media sources: versions the user cannot
// access are not selectable, so they must not count towards the badge either.
var mediaSourceCount = user is null
? video.MediaSourceCount
: video.GetAllVersions().Count(v => v.Id.Equals(video.Id) || v.IsVisibleStandalone(user));
|| (!video.PrimaryVersionId.HasValue && video.LinkedAlternateVersions.Length == 0 && !video.HasLocalAlternateVersions)
? video.MediaSourceCount
: video.GetAllVersions().Count(v => v.Id.Equals(video.Id) || v.IsVisibleStandalone(user));
if (mediaSourceCount != 1)
{
dto.MediaSourceCount = mediaSourceCount;

25
Emby.Server.Implementations/Library/MediaSourceManager.cs
View File

@@ -451,26 +451,23 @@ namespace Emby.Server.Implementations.Library
}
}
MediaSourceInfo resumeSource = null;
UserItemData resumeData = null;
foreach (var source in sources)
{
if (source.Id is null
|| !dataBySourceId.TryGetValue(source.Id, out var data)
|| data.PlaybackPositionTicks <= 0)
if (source.Id is not null
&& dataBySourceId.TryGetValue(source.Id, out var data)
&& data.PlaybackPositionTicks > 0)
{
continue;
}
source.PlaybackPositionTicks = data.PlaybackPositionTicks;
if (resumeData is null || (data.LastPlayedDate ?? DateTime.MinValue) > (resumeData.LastPlayedDate ?? DateTime.MinValue))
{
resumeSource = source;
resumeData = data;
source.PlaybackPositionTicks = data.PlaybackPositionTicks;
}
}
// Reorder only for a resumable (in-progress) version;
// a completed version has no position to resume, so it must not be pulled to the front here.
var resumeSource = VersionPlaybackSelector.SelectMostRecentlyPlayed(
sources,
source => source.Id is not null ? dataBySourceId.GetValueOrDefault(source.Id) : null,
data => data.PlaybackPositionTicks > 0);
if (resumeSource is not null && !video.PrimaryVersionId.HasValue && !ReferenceEquals(sources[0], resumeSource))
{
var reordered = new List<MediaSourceInfo>(sources.Count) { resumeSource };

20
Emby.Server.Implementations/Library/UserDataManager.cs
View File

@@ -329,21 +329,15 @@ namespace Emby.Server.Implementations.Library
foreach (var (primaryId, versions) in versionGroups)
{
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.Played)
&& (resumeData is null || (data.LastPlayedDate ?? DateTime.MinValue) > (resumeData.LastPlayedDate ?? DateTime.MinValue)))
{
resumeData = data;
}
}
// Consider both in-progress and completed versions so a finished alternate still marks the primary as played.
var resumeVersion = VersionPlaybackSelector.SelectMostRecentlyPlayed(
versions,
version => userDataByVersion.GetValueOrDefault(version.Id),
data => data.PlaybackPositionTicks > 0 || data.Played);
if (resumeData is not null)
if (resumeVersion is not null)
{
result[primaryId] = new VersionResumeData(resumeData);
result[primaryId] = new VersionResumeData(resumeVersion.Id, userDataByVersion[resumeVersion.Id]);
}
}

11
Emby.Server.Implementations/Session/SessionManager.cs
View File

@@ -980,14 +980,17 @@ namespace Emby.Server.Implementations.Session
{
_userDataManager.SaveUserData(user, item, data, UserDataSaveReason.PlaybackProgress, CancellationToken.None);
// A completed version marks every alternate version played and clears their resume points, so the
// whole movie leaves Continue Watching and reads as watched everywhere. (Per-version resume positions
// only persist while nothing has been completed yet.)
if (data.Played == true && item is Video playedVideo)
{
playedVideo.PropagatePlayedState(user, true);
}
}
if ((!user.RememberAudioSelections && data.AudioStreamIndex.HasValue)
|| (!user.RememberSubtitleSelections && data.SubtitleStreamIndex.HasValue))
if ((!user.RememberAudioSelections && info.AudioStreamIndex.HasValue)
|| (!user.RememberSubtitleSelections && info.SubtitleStreamIndex.HasValue))
{
_userDataManager.ResetPlaybackStreamSelections(user, item);
}
@@ -1177,7 +1180,9 @@ namespace Emby.Server.Implementations.Session
_userDataManager.SaveUserData(user, item, data, UserDataSaveReason.PlaybackFinished, CancellationToken.None);
// A completed version marks all of its alternate versions played; positions stay per-version.
// A completed version marks every alternate version played and clears their resume points, so the
// whole movie leaves Continue Watching and reads as watched everywhere. (Per-version resume positions
// only persist while nothing has been completed yet.)
if (data.Played == true && item is Video playedVideo)
{
playedVideo.PropagatePlayedState(user, true);

20
Emby.Server.Implementations/TV/TVSeriesManager.cs
View File

@@ -262,19 +262,15 @@ namespace Emby.Server.Implementations.TV
return (null, null);
}
Video? playedVersion = null;
DateTime? lastPlayedDate = null;
foreach (var version in lastWatchedVideo.GetAllVersions())
{
var data = _userDataManager.GetUserData(user, version);
if (data?.LastPlayedDate is { } date && (lastPlayedDate is null || date > lastPlayedDate))
{
lastPlayedDate = date;
playedVersion = version;
}
}
var versions = lastWatchedVideo.GetAllVersions();
var userDataByVersion = _userDataManager.GetUserDataBatch(versions, user);
return (playedVersion, lastPlayedDate);
var playedVersion = VersionPlaybackSelector.SelectMostRecentlyPlayed(
versions,
version => userDataByVersion.GetValueOrDefault(version.Id),
data => data.LastPlayedDate.HasValue);
return (playedVersion, playedVersion is null ? null : userDataByVersion[playedVersion.Id].LastPlayedDate);
}
/// <summary>

8
Jellyfin.Api/Controllers/ItemsController.cs
View File

@@ -963,9 +963,15 @@ public class ItemsController : BaseJellyfinApiController
var excludeItemIds = Array.Empty<Guid>();
if (excludeActiveSessions)
{
// NowPlayingItem.Id is the displayed/primary id, but resume queries surface the actually-played
// alternate version's own id. Expand each active session to every version id so an in-progress
// alternate is excluded too, instead of leaking back into the resume list.
excludeItemIds = _sessionManager.Sessions
.Where(s => s.UserId.Equals(requestUserId) && s.NowPlayingItem is not null)
.Select(s => s.NowPlayingItem.Id)
.SelectMany(s => _libraryManager.GetItemById(s.NowPlayingItem.Id) is Video video
? video.GetAllVersions().Select(v => v.Id)
: [s.NowPlayingItem.Id])
.Distinct()
.ToArray();
}

10
Jellyfin.Server.Implementations/Item/BaseItemRepository.TranslateQuery.cs
View File

@@ -556,11 +556,15 @@ public sealed partial class BaseItemRepository
|| (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.
// When several versions of the same item are in progress, keep only the most recently played one, use id as tiebreaker.
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)));
.Any(s =>
inProgress.Where(su => su.ItemId == s.Id).Max(su => su.LastPlayedDate)
> inProgress.Where(eu => eu.ItemId == e.Id).Max(eu => eu.LastPlayedDate)
|| (inProgress.Where(su => su.ItemId == s.Id).Max(su => su.LastPlayedDate)
== inProgress.Where(eu => eu.ItemId == e.Id).Max(eu => eu.LastPlayedDate)
&& s.Id.CompareTo(e.Id) < 0)));
}
else
{

4
Jellyfin.Server.Implementations/Item/OrderMapper.cs
View File

@@ -38,6 +38,10 @@ public static class OrderMapper
jellyfinDbContext.UserData
.Where(w => w.UserId == query.User.Id && (w.ItemId == e.Id || w.Item!.PrimaryVersionId == e.Id))
.Max(f => f.LastPlayedDate),
(ItemSortBy.DatePlayed, null) => e =>
jellyfinDbContext.UserData
.Where(w => w.ItemId == e.Id || w.Item!.PrimaryVersionId == e.Id)
.Max(f => f.LastPlayedDate),
(ItemSortBy.PlayCount, _) => e => e.UserData!.Where(f => f.UserId.Equals(query.User!.Id)).OrderBy(f => f.CustomDataKey).FirstOrDefault()!.PlayCount,
(ItemSortBy.IsFavoriteOrLiked, _) => e => e.UserData!.Where(f => f.UserId.Equals(query.User!.Id)).OrderBy(f => f.CustomDataKey).Select(f => (bool?)f.IsFavorite).FirstOrDefault() ?? false,
(ItemSortBy.IsFolder, _) => e => e.IsFolder,

59
MediaBrowser.Controller/Library/VersionPlaybackSelector.cs Normal file
View File

@@ -0,0 +1,59 @@
using System;
using System.Collections.Generic;
using MediaBrowser.Controller.Entities;
namespace MediaBrowser.Controller.Library
{
/// <summary>
/// Single definition of "which alternate version was most recently played" shared by the resume tile
/// (<see cref="IUserDataManager.GetResumeUserData"/>), the media-source default ordering and Next Up.
/// Each call site declares its own eligibility rule so the intentional differences (resumable-only vs.
/// resumable-or-completed) are visible in one place instead of being re-implemented divergently.
/// The SQL resume query keeps its own translation of the same rule.
/// </summary>
public static class VersionPlaybackSelector
{
/// <summary>
/// Selects the entry whose user data has the greatest <see cref="UserItemData.LastPlayedDate"/>,
/// considering only entries that satisfy <paramref name="isEligible"/>. On an exact tie the first
/// encountered entry wins.
/// </summary>
/// <typeparam name="T">The candidate type (e.g. a version item or a media source).</typeparam>
/// <param name="items">The candidates to choose from.</param>
/// <param name="dataSelector">Resolves the user data for a candidate, or <c>null</c> when it has none.</param>
/// <param name="isEligible">Whether a candidate's user data makes it a valid winner.</param>
/// <returns>The most recently played eligible candidate, or <c>default</c> when none qualify.</returns>
public static T? SelectMostRecentlyPlayed<T>(
IEnumerable<T> items,
Func<T, UserItemData?> dataSelector,
Func<UserItemData, bool> isEligible)
{
ArgumentNullException.ThrowIfNull(items);
ArgumentNullException.ThrowIfNull(dataSelector);
ArgumentNullException.ThrowIfNull(isEligible);
T? winner = default;
var winnerDate = DateTime.MinValue;
var hasWinner = false;
foreach (var item in items)
{
var data = dataSelector(item);
if (data is null || !isEligible(data))
{
continue;
}
var date = data.LastPlayedDate ?? DateTime.MinValue;
if (!hasWinner || date > winnerDate)
{
winner = item;
winnerDate = date;
hasWinner = true;
}
}
return winner;
}
}
}

19
MediaBrowser.Controller/Library/VersionResumeData.cs
View File

@@ -7,14 +7,17 @@ namespace MediaBrowser.Controller.Library
/// <summary>
/// The user data of the most recently played alternate version that should drive the completion state of a multi-version item.
/// </summary>
/// <param name="VersionId">The id of the version that owns <paramref name="UserData"/>.</param>
/// <param name="UserData">The resume version's user data.</param>
public record VersionResumeData(UserItemData UserData)
public record VersionResumeData(Guid VersionId, UserItemData UserData)
{
/// <summary>
/// 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.
/// Completion (played) propagates to the primary. An 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. When the movie was
/// finished on a different version, the primary's own stale resume position is cleared so it does not
/// render as "watched and resumable" at the same time.
/// </summary>
/// <param name="dto">The user data dto to update.</param>
public void ApplyTo(UserItemDataDto dto)
@@ -25,6 +28,14 @@ namespace MediaBrowser.Controller.Library
{
dto.LastPlayedDate = UserData.LastPlayedDate;
}
// A different version was finished (played, no resume position of its own) and is the most
// recently played: the whole movie is watched.
if (!VersionId.Equals(dto.ItemId) && UserData.Played && UserData.PlaybackPositionTicks <= 0)
{
dto.PlaybackPositionTicks = 0;
dto.PlayedPercentage = null;
}
}
}
}

53
tests/Jellyfin.Controller.Tests/Library/VersionResumeDataTests.cs
View File

@@ -9,13 +9,14 @@ namespace Jellyfin.Controller.Tests.Library;
public class VersionResumeDataTests
{
[Fact]
public void ApplyTo_PropagatesCompletionButNotPosition()
public void ApplyTo_CompletedOtherVersion_PropagatesCompletionAndClearsStaleResume()
{
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 });
Guid.NewGuid(),
new UserItemData { Key = "version", PlaybackPositionTicks = 0, Played = true, LastPlayedDate = lastPlayed });
var dto = new UserItemDataDto { Key = "primary", PlaybackPositionTicks = 1, Played = false, PlayedPercentage = 1 };
var dto = new UserItemDataDto { ItemId = Guid.NewGuid(), Key = "primary", PlaybackPositionTicks = 1, Played = false, PlayedPercentage = 50 };
resume.ApplyTo(dto);
@@ -23,9 +24,48 @@ public class VersionResumeDataTests
Assert.True(dto.Played);
Assert.Equal(lastPlayed, dto.LastPlayedDate);
// ...but the in-progress resume position stays on the version that owns it.
// ...and because the movie was finished on a different version, the primary's own stale resume bar is cleared.
Assert.Equal(0, dto.PlaybackPositionTicks);
Assert.Null(dto.PlayedPercentage);
}
[Fact]
public void ApplyTo_PrimaryOwnProgress_KeepsResumePosition()
{
var primaryId = Guid.NewGuid();
var lastPlayed = new DateTime(2026, 1, 2, 0, 0, 0, DateTimeKind.Utc);
// The winning version is the primary itself (e.g. rewatching): its resume bar must survive.
var resume = new VersionResumeData(
primaryId,
new UserItemData { Key = "primary", PlaybackPositionTicks = 5, Played = true, LastPlayedDate = lastPlayed });
var dto = new UserItemDataDto { ItemId = primaryId, Key = "primary", PlaybackPositionTicks = 5, Played = true, PlayedPercentage = 20 };
resume.ApplyTo(dto);
Assert.True(dto.Played);
Assert.Equal(5, dto.PlaybackPositionTicks);
Assert.Equal(20, dto.PlayedPercentage);
}
[Fact]
public void ApplyTo_InProgressOtherVersion_KeepsPrimaryResumePosition()
{
var lastPlayed = new DateTime(2026, 1, 2, 0, 0, 0, DateTimeKind.Utc);
// A different version that is in-progress (not finished) must not clear the primary's position.
var resume = new VersionResumeData(
Guid.NewGuid(),
new UserItemData { Key = "version", PlaybackPositionTicks = 25, Played = false, LastPlayedDate = lastPlayed });
var dto = new UserItemDataDto { ItemId = Guid.NewGuid(), Key = "primary", PlaybackPositionTicks = 1, Played = false, PlayedPercentage = 50 };
resume.ApplyTo(dto);
Assert.False(dto.Played);
Assert.Equal(1, dto.PlaybackPositionTicks);
Assert.Equal(1.0, dto.PlayedPercentage);
Assert.Equal(50, dto.PlayedPercentage);
}
[Fact]
@@ -34,9 +74,10 @@ public class VersionResumeDataTests
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(
Guid.NewGuid(),
new UserItemData { Key = "version", Played = false, LastPlayedDate = versionLastPlayed });
var dto = new UserItemDataDto { Key = "primary", Played = true, LastPlayedDate = primaryLastPlayed };
var dto = new UserItemDataDto { ItemId = Guid.NewGuid(), Key = "primary", Played = true, LastPlayedDate = primaryLastPlayed };
resume.ApplyTo(dto);

85
tests/Jellyfin.Server.Implementations.Tests/Item/AlternateVersionQueryTranslationTests.cs
View File

@@ -61,8 +61,12 @@ public sealed class AlternateVersionQueryTranslationTests : IDisposable
.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)))
.Any(s =>
inProgress.Where(su => su.ItemId == s.Id).Max(su => su.LastPlayedDate)
> inProgress.Where(eu => eu.ItemId == e.Id).Max(eu => eu.LastPlayedDate)
|| (inProgress.Where(su => su.ItemId == s.Id).Max(su => su.LastPlayedDate)
== inProgress.Where(eu => eu.ItemId == e.Id).Max(eu => eu.LastPlayedDate)
&& s.Id.CompareTo(e.Id) < 0)))
.Select(e => e.Id)
.ToList();
@@ -81,6 +85,46 @@ public sealed class AlternateVersionQueryTranslationTests : IDisposable
}
}
[Fact]
public void ResumeFilter_TiedLastPlayedDate_KeepsSingleVersion()
{
Guid userId, primaryId, versionAId, versionBId;
using (var ctx = CreateDbContext())
{
(userId, primaryId, versionAId, versionBId) = SeedTiedVersions(ctx);
}
using (var ctx = CreateDbContext())
{
var inProgress = ctx.UserData
.Where(ud => ud.UserId == userId && ud.PlaybackPositionTicks > 0);
var seededIds = new[] { primaryId, versionAId, versionBId };
var inProgressIds = inProgress.Select(ud => ud.ItemId);
// The exact production dedup, including the Guid.CompareTo tie-break. This asserts the
// expression translates on SQLite and that two versions sharing an identical LastPlayedDate
// collapse to a single row instead of double-listing the item in Continue Watching.
var resumable = ctx.BaseItems
.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)
|| (inProgress.Where(su => su.ItemId == s.Id).Max(su => su.LastPlayedDate)
== inProgress.Where(eu => eu.ItemId == e.Id).Max(eu => eu.LastPlayedDate)
&& s.Id.CompareTo(e.Id) < 0)))
.Select(e => e.Id)
.ToList();
var survivor = Assert.Single(resumable);
Assert.Contains(survivor, new[] { versionAId, versionBId });
}
}
[Fact]
public void DatePlayedOrdering_VersionProgress_SortsPrimaryByVersionDate()
{
@@ -136,6 +180,43 @@ public sealed class AlternateVersionQueryTranslationTests : IDisposable
return (user.Id, primary.Id, version.Id, other.Id);
}
private static (Guid UserId, Guid PrimaryId, Guid VersionAId, Guid VersionBId) SeedTiedVersions(JellyfinDbContext ctx)
{
var user = new User("test", "auth-provider", "reset-provider");
ctx.Users.Add(user);
var primary = new BaseItemEntity { Id = Guid.NewGuid(), Type = "MediaBrowser.Controller.Entities.Movies.Movie" };
var versionA = new BaseItemEntity { Id = Guid.NewGuid(), Type = "MediaBrowser.Controller.Entities.Movies.Movie", PrimaryVersionId = primary.Id };
var versionB = new BaseItemEntity { Id = Guid.NewGuid(), Type = "MediaBrowser.Controller.Entities.Movies.Movie", PrimaryVersionId = primary.Id };
ctx.BaseItems.AddRange(primary, versionA, versionB);
// Both versions in progress with the exact same LastPlayedDate - the tie that a strict '>' cannot break.
var tied = new DateTime(2026, 1, 2, 0, 0, 0, DateTimeKind.Utc);
ctx.UserData.Add(new UserData
{
ItemId = versionA.Id,
Item = versionA,
UserId = user.Id,
User = user,
CustomDataKey = versionA.Id.ToString("N"),
PlaybackPositionTicks = 1000,
LastPlayedDate = tied
});
ctx.UserData.Add(new UserData
{
ItemId = versionB.Id,
Item = versionB,
UserId = user.Id,
User = user,
CustomDataKey = versionB.Id.ToString("N"),
PlaybackPositionTicks = 2000,
LastPlayedDate = tied
});
ctx.SaveChanges();
return (user.Id, primary.Id, versionA.Id, versionB.Id);
}
private JellyfinDbContext CreateDbContext()
{
return new JellyfinDbContext(