Merge pull request #17631 from itsb/fix/confirmed-resume-position-master

Use client-reported position for idle playback cleanup

4 files changed, 191 insertions, 1 deletions

diff --git a/Emby.Server.Implementations/Session/SessionManager.cs b/Emby.Server.Implementations/Session/SessionManager.cs
index 5d62332552..f4aa0ad03a 100644
--- a/Emby.Server.Implementations/Session/SessionManager.cs
+++ b/Emby.Server.Implementations/Session/SessionManager.cs
@@ -655,7 +655,7 @@ namespace Emby.Server.Implementations.Session
                             ItemId = session.NowPlayingItem is null ? Guid.Empty : session.NowPlayingItem.Id,
                             SessionId = session.Id,
                             MediaSourceId = session.PlayState?.MediaSourceId,
-                            PositionTicks = session.PlayState?.PositionTicks
+                            PositionTicks = session.LastPlaybackCheckInPositionTicks
                         }).ConfigureAwait(false);
                     }
                     catch (Exception ex)
diff --git a/MediaBrowser.Controller/Session/SessionInfo.cs b/MediaBrowser.Controller/Session/SessionInfo.cs
index fb68bfb770..d9bc79dec5 100644
--- a/MediaBrowser.Controller/Session/SessionInfo.cs
+++ b/MediaBrowser.Controller/Session/SessionInfo.cs
@@ -28,6 +28,7 @@ namespace MediaBrowser.Controller.Session
         private readonly Lock _progressLock = new();
         private Timer _progressTimer;
         private PlaybackProgressInfo _lastProgressInfo;
+        private long? _lastPlaybackCheckInPositionTicks;
 
         private bool _disposed;
 
@@ -125,6 +126,22 @@ namespace MediaBrowser.Controller.Session
         public DateTime LastPlaybackCheckIn { get; set; }
 
         /// <summary>
+        /// Gets the position reported by the client at the last playback check-in.
+        /// </summary>
+        /// <value>The position ticks, or <see langword="null"/> if the client did not report a position.</value>
+        [JsonIgnore]
+        public long? LastPlaybackCheckInPositionTicks
+        {
+            get
+            {
+                lock (_progressLock)
+                {
+                    return _lastPlaybackCheckInPositionTicks;
+                }
+            }
+        }
+
+        /// <summary>
         /// Gets or sets the last paused date.
         /// </summary>
         /// <value>The last paused date.</value>
@@ -372,6 +389,7 @@ namespace MediaBrowser.Controller.Session
 
             lock (_progressLock)
             {
+                _lastPlaybackCheckInPositionTicks = progressInfo.PositionTicks;
                 _lastProgressInfo = progressInfo;
 
                 if (_progressTimer is null)
diff --git a/tests/Jellyfin.Server.Implementations.Tests/SessionManager/IdlePlaybackTests.cs b/tests/Jellyfin.Server.Implementations.Tests/SessionManager/IdlePlaybackTests.cs
new file mode 100644
index 0000000000..7722707cbe
--- /dev/null
+++ b/tests/Jellyfin.Server.Implementations.Tests/SessionManager/IdlePlaybackTests.cs
@@ -0,0 +1,80 @@
+using System;
+using System.Reflection;
+using System.Threading.Tasks;
+using MediaBrowser.Controller;
+using MediaBrowser.Controller.Configuration;
+using MediaBrowser.Controller.Devices;
+using MediaBrowser.Controller.Drawing;
+using MediaBrowser.Controller.Dto;
+using MediaBrowser.Controller.Events;
+using MediaBrowser.Controller.Library;
+using MediaBrowser.Controller.Session;
+using MediaBrowser.Model.Dto;
+using MediaBrowser.Model.Session;
+using Microsoft.Extensions.Hosting;
+using Microsoft.Extensions.Logging.Abstractions;
+using Moq;
+using Xunit;
+
+namespace Jellyfin.Server.Implementations.Tests.SessionManager;
+
+public class IdlePlaybackTests
+{
+    [Theory]
+    [InlineData(null, null)]
+    [InlineData(123456789L, 123456789L)]
+    public async Task CheckForIdlePlayback_StopsAtLastClientReportedPosition(long? clientPositionTicks, long? expectedPositionTicks)
+    {
+        var playbackStopped = new TaskCompletionSource<long?>(TaskCreationOptions.RunContinuationsAsynchronously);
+        var eventManager = new Mock<IEventManager>();
+        eventManager
+            .Setup(manager => manager.PublishAsync(It.IsAny<PlaybackStopEventArgs>()))
+            .Callback<PlaybackStopEventArgs>(eventArgs => playbackStopped.TrySetResult(eventArgs.PlaybackPositionTicks))
+            .Returns(Task.CompletedTask);
+        await using var sessionManager = new Emby.Server.Implementations.Session.SessionManager(
+            NullLogger<Emby.Server.Implementations.Session.SessionManager>.Instance,
+            eventManager.Object,
+            Mock.Of<IUserDataManager>(),
+            Mock.Of<IServerConfigurationManager>(),
+            Mock.Of<ILibraryManager>(),
+            Mock.Of<IUserManager>(),
+            Mock.Of<IMusicManager>(),
+            Mock.Of<IDtoService>(),
+            Mock.Of<IImageProcessor>(),
+            Mock.Of<IServerApplicationHost>(),
+            Mock.Of<IDeviceManager>(),
+            Mock.Of<IMediaSourceManager>(),
+            Mock.Of<IHostApplicationLifetime>());
+        var session = await sessionManager.LogSessionActivity(
+            "Test Client",
+            "1.0.0",
+            "test-device",
+            "Test Device",
+            "127.0.0.1",
+            null);
+        session.NowPlayingItem = new BaseItemDto
+        {
+            Id = Guid.NewGuid(),
+            Name = "Test Item"
+        };
+        session.PlayState.PositionTicks = 987654321;
+
+        if (clientPositionTicks.HasValue)
+        {
+            session.StartAutomaticProgress(new PlaybackProgressInfo
+            {
+                IsPaused = true,
+                PositionTicks = clientPositionTicks
+            });
+            session.StopAutomaticProgress();
+        }
+
+        var idlePlaybackCallback = typeof(Emby.Server.Implementations.Session.SessionManager)
+            .GetMethod("CheckForIdlePlayback", BindingFlags.Instance | BindingFlags.NonPublic)!;
+        idlePlaybackCallback.Invoke(sessionManager, new object?[] { null });
+
+        var stoppedPositionTicks = await playbackStopped.Task.WaitAsync(TimeSpan.FromSeconds(5), TestContext.Current.CancellationToken);
+
+        Assert.Equal(expectedPositionTicks, stoppedPositionTicks);
+    }
+}
diff --git a/tests/Jellyfin.Server.Implementations.Tests/SessionManager/SessionInfoTests.cs b/tests/Jellyfin.Server.Implementations.Tests/SessionManager/SessionInfoTests.cs
new file mode 100644
index 0000000000..c5b8f661b5
--- /dev/null
+++ b/tests/Jellyfin.Server.Implementations.Tests/SessionManager/SessionInfoTests.cs
@@ -0,0 +1,92 @@
+using System;
+using System.Threading.Tasks;
+using MediaBrowser.Controller.Session;
+using MediaBrowser.Model.Session;
+using Microsoft.Extensions.Logging.Abstractions;
+using Moq;
+using Xunit;
+
+namespace Jellyfin.Server.Implementations.Tests.SessionManager;
+
+public class SessionInfoTests
+{
+    [Fact]
+    public async Task StartAutomaticProgress_SnapshotsClientReportedPosition()
+    {
+        await using var session = new SessionInfo(Mock.Of<ISessionManager>(), NullLogger.Instance);
+        var progressInfo = new PlaybackProgressInfo
+        {
+            IsPaused = true,
+            PositionTicks = 123456789
+        };
+
+        session.StartAutomaticProgress(progressInfo);
+
+        Assert.Equal(progressInfo.PositionTicks, session.LastPlaybackCheckInPositionTicks);
+    }
+
+    [Fact]
+    public async Task AutomaticProgress_AdvancesEstimatedPositionWithoutAdvancingSnapshot()
+    {
+        var sessionManager = new Mock<ISessionManager>();
+        await using var session = new SessionInfo(sessionManager.Object, NullLogger.Instance);
+        var automaticProgress = new TaskCompletionSource<long?>(TaskCreationOptions.RunContinuationsAsynchronously);
+        const long reportedPositionTicks = 123456789;
+
+        sessionManager
+            .Setup(manager => manager.OnPlaybackProgress(It.IsAny<PlaybackProgressInfo>(), true))
+            .Callback<PlaybackProgressInfo, bool>((info, _) =>
+            {
+                session.PlayState.PositionTicks = info.PositionTicks;
+                automaticProgress.TrySetResult(info.PositionTicks);
+            })
+            .Returns(Task.CompletedTask);
+        session.PlayState.PositionTicks = reportedPositionTicks;
+
+        session.StartAutomaticProgress(new PlaybackProgressInfo
+        {
+            PositionTicks = reportedPositionTicks
+        });
+
+        var estimatedPositionTicks = await automaticProgress.Task.WaitAsync(TimeSpan.FromSeconds(5), TestContext.Current.CancellationToken);
+        session.StopAutomaticProgress();
+
+        Assert.Equal(reportedPositionTicks + TimeSpan.TicksPerSecond, estimatedPositionTicks);
+        Assert.Equal(estimatedPositionTicks, session.PlayState.PositionTicks);
+        Assert.Equal(reportedPositionTicks, session.LastPlaybackCheckInPositionTicks);
+    }
+
+    [Fact]
+    public async Task StartAutomaticProgress_ReplacesSnapshotOnLaterClientReport()
+    {
+        await using var session = new SessionInfo(Mock.Of<ISessionManager>(), NullLogger.Instance);
+        session.StartAutomaticProgress(new PlaybackProgressInfo
+        {
+            IsPaused = true,
+            PositionTicks = 123456789
+        });
+
+        session.StartAutomaticProgress(new PlaybackProgressInfo
+        {
+            IsPaused = true,
+            PositionTicks = 987654321
+        });
+
+        Assert.Equal(987654321, session.LastPlaybackCheckInPositionTicks);
+    }
+
+    [Fact]
+    public async Task StartAutomaticProgress_PreservesExactPausedPosition()
+    {
+        await using var session = new SessionInfo(Mock.Of<ISessionManager>(), NullLogger.Instance);
+        var pausedProgress = new PlaybackProgressInfo
+        {
+            IsPaused = true,
+            PositionTicks = 314159265
+        };
+
+        session.StartAutomaticProgress(pausedProgress);
+
+        Assert.Equal(pausedProgress.PositionTicks, session.LastPlaybackCheckInPositionTicks);
+    }
+}