From 0c735a039538bfaa44d25df415f3d41519e37194 Mon Sep 17 00:00:00 2001 From: Ionut Andrei Oanca Date: Thu, 22 Oct 2020 15:51:58 +0200 Subject: [PATCH] Address requested changes by review --- .../SyncPlay/GroupController.cs | 50 ++++++++++--------- .../GroupStates/AbstractGroupState.cs | 2 +- .../SyncPlay/GroupStates/IdleGroupState.cs | 3 +- .../SyncPlay/GroupStates/PausedGroupState.cs | 9 +++- .../SyncPlay/GroupStates/PlayingGroupState.cs | 3 +- .../SyncPlay/GroupStates/WaitingGroupState.cs | 17 +++++-- .../SyncPlay/Queue/PlayQueueManager.cs | 25 +--------- 7 files changed, 54 insertions(+), 55 deletions(-) diff --git a/Emby.Server.Implementations/SyncPlay/GroupController.cs b/Emby.Server.Implementations/SyncPlay/GroupController.cs index 687aa7a3a..65a711fb4 100644 --- a/Emby.Server.Implementations/SyncPlay/GroupController.cs +++ b/Emby.Server.Implementations/SyncPlay/GroupController.cs @@ -126,16 +126,6 @@ namespace Emby.Server.Implementations.SyncPlay State = new IdleGroupState(_logger); } - /// - /// Checks if a session is in this group. - /// - /// The session identifier to check. - /// true if the session is in this group; false otherwise. - private bool ContainsSession(string sessionId) - { - return Participants.ContainsKey(sessionId); - } - /// /// Adds the session to the group. /// @@ -174,16 +164,22 @@ namespace Emby.Server.Implementations.SyncPlay case SyncPlayBroadcastType.CurrentSession: return new SessionInfo[] { from }; case SyncPlayBroadcastType.AllGroup: - return Participants.Values.Select( - session => session.Session).ToArray(); + return Participants + .Values + .Select(session => session.Session) + .ToArray(); case SyncPlayBroadcastType.AllExceptCurrentSession: - return Participants.Values.Select( - session => session.Session).Where( - session => !session.Id.Equals(from.Id)).ToArray(); + return Participants + .Values + .Select(session => session.Session) + .Where(session => !session.Id.Equals(from.Id)) + .ToArray(); case SyncPlayBroadcastType.AllReady: - return Participants.Values.Where( - session => !session.IsBuffering).Select( - session => session.Session).ToArray(); + return Participants + .Values + .Where(session => !session.IsBuffering) + .Select(session => session.Session) + .ToArray(); default: return Array.Empty(); } @@ -236,7 +232,8 @@ namespace Emby.Server.Implementations.SyncPlay } // Get list of users. - var users = Participants.Values + var users = Participants + .Values .Select(participant => _userManager.GetUserById(participant.Session.UserId)); // Find problematic users. @@ -365,7 +362,7 @@ namespace Emby.Server.Implementations.SyncPlay /// public void SetIgnoreGroupWait(SessionInfo session, bool ignoreGroupWait) { - if (!ContainsSession(session.Id)) + if (!Participants.ContainsKey(session.Id)) { return; } @@ -443,8 +440,8 @@ namespace Emby.Server.Implementations.SyncPlay public long SanitizePositionTicks(long? positionTicks) { var ticks = positionTicks ?? 0; - ticks = ticks >= 0 ? ticks : 0; - ticks = ticks > RunTimeTicks ? RunTimeTicks : ticks; + ticks = Math.Max(ticks, 0); + ticks = Math.Min(ticks, RunTimeTicks); return ticks; } @@ -663,8 +660,13 @@ namespace Emby.Server.Implementations.SyncPlay { var currentTime = DateTime.UtcNow; var elapsedTime = currentTime - LastActivity; - // Event may happen during the delay added to account for latency. - startPositionTicks += elapsedTime.Ticks > 0 ? elapsedTime.Ticks : 0; + // Elapsed time is negative if event happens + // during the delay added to account for latency. + // In this phase clients haven't started the playback yet. + // In other words, LastActivity is in the future, + // when playback unpause is supposed to happen. + // Adjust ticks only if playback actually started. + startPositionTicks += Math.Max(elapsedTime.Ticks, 0); } return new PlayQueueUpdate() diff --git a/Emby.Server.Implementations/SyncPlay/GroupStates/AbstractGroupState.cs b/Emby.Server.Implementations/SyncPlay/GroupStates/AbstractGroupState.cs index 1eb110772..26cd51b8d 100644 --- a/Emby.Server.Implementations/SyncPlay/GroupStates/AbstractGroupState.cs +++ b/Emby.Server.Implementations/SyncPlay/GroupStates/AbstractGroupState.cs @@ -212,7 +212,7 @@ namespace MediaBrowser.Controller.SyncPlay private void UnhandledRequest(IPlaybackGroupRequest request) { - _logger.LogWarning("HandleRequest: unhandled {0} request for {1} state.", request.GetRequestType(), this.GetGroupState()); + _logger.LogWarning("HandleRequest: unhandled {0} request for {1} state.", request.GetRequestType(), GetGroupState()); } } } diff --git a/Emby.Server.Implementations/SyncPlay/GroupStates/IdleGroupState.cs b/Emby.Server.Implementations/SyncPlay/GroupStates/IdleGroupState.cs index b8510715a..70fe3e006 100644 --- a/Emby.Server.Implementations/SyncPlay/GroupStates/IdleGroupState.cs +++ b/Emby.Server.Implementations/SyncPlay/GroupStates/IdleGroupState.cs @@ -16,7 +16,8 @@ namespace MediaBrowser.Controller.SyncPlay /// /// Default constructor. /// - public IdleGroupState(ILogger logger) : base(logger) + public IdleGroupState(ILogger logger) + : base(logger) { // Do nothing. } diff --git a/Emby.Server.Implementations/SyncPlay/GroupStates/PausedGroupState.cs b/Emby.Server.Implementations/SyncPlay/GroupStates/PausedGroupState.cs index 8c4bd20b1..ca2cb0988 100644 --- a/Emby.Server.Implementations/SyncPlay/GroupStates/PausedGroupState.cs +++ b/Emby.Server.Implementations/SyncPlay/GroupStates/PausedGroupState.cs @@ -17,7 +17,8 @@ namespace MediaBrowser.Controller.SyncPlay /// /// Default constructor. /// - public PausedGroupState(ILogger logger) : base(logger) + public PausedGroupState(ILogger logger) + : base(logger) { // Do nothing. } @@ -70,8 +71,12 @@ namespace MediaBrowser.Controller.SyncPlay var currentTime = DateTime.UtcNow; var elapsedTime = currentTime - context.LastActivity; context.LastActivity = currentTime; + // Elapsed time is negative if event happens + // during the delay added to account for latency. + // In this phase clients haven't started the playback yet. + // In other words, LastActivity is in the future, + // when playback unpause is supposed to happen. // Seek only if playback actually started. - // Pause request may be issued during the delay added to account for latency. context.PositionTicks += Math.Max(elapsedTime.Ticks, 0); var command = context.NewSyncPlayCommand(SendCommandType.Pause); diff --git a/Emby.Server.Implementations/SyncPlay/GroupStates/PlayingGroupState.cs b/Emby.Server.Implementations/SyncPlay/GroupStates/PlayingGroupState.cs index a3b0baf96..85119669d 100644 --- a/Emby.Server.Implementations/SyncPlay/GroupStates/PlayingGroupState.cs +++ b/Emby.Server.Implementations/SyncPlay/GroupStates/PlayingGroupState.cs @@ -22,7 +22,8 @@ namespace MediaBrowser.Controller.SyncPlay /// /// Default constructor. /// - public PlayingGroupState(ILogger logger) : base(logger) + public PlayingGroupState(ILogger logger) + : base(logger) { // Do nothing. } diff --git a/Emby.Server.Implementations/SyncPlay/GroupStates/WaitingGroupState.cs b/Emby.Server.Implementations/SyncPlay/GroupStates/WaitingGroupState.cs index ff1d379d7..8e970751f 100644 --- a/Emby.Server.Implementations/SyncPlay/GroupStates/WaitingGroupState.cs +++ b/Emby.Server.Implementations/SyncPlay/GroupStates/WaitingGroupState.cs @@ -32,7 +32,8 @@ namespace MediaBrowser.Controller.SyncPlay /// /// Default constructor. /// - public WaitingGroupState(ILogger logger) : base(logger) + public WaitingGroupState(ILogger logger) + : base(logger) { // Do nothing. } @@ -59,8 +60,12 @@ namespace MediaBrowser.Controller.SyncPlay var currentTime = DateTime.UtcNow; var elapsedTime = currentTime - context.LastActivity; context.LastActivity = currentTime; + // Elapsed time is negative if event happens + // during the delay added to account for latency. + // In this phase clients haven't started the playback yet. + // In other words, LastActivity is in the future, + // when playback unpause is supposed to happen. // Seek only if playback actually started. - // Event may happen during the delay added to account for latency. context.PositionTicks += Math.Max(elapsedTime.Ticks, 0); } @@ -355,6 +360,12 @@ namespace MediaBrowser.Controller.SyncPlay var currentTime = DateTime.UtcNow; var elapsedTime = currentTime - context.LastActivity; context.LastActivity = currentTime; + // Elapsed time is negative if event happens + // during the delay added to account for latency. + // In this phase clients haven't started the playback yet. + // In other words, LastActivity is in the future, + // when playback unpause is supposed to happen. + // Seek only if playback actually started. context.PositionTicks += Math.Max(elapsedTime.Ticks, 0); // Send pause command to all non-buffering sessions. @@ -484,7 +495,7 @@ namespace MediaBrowser.Controller.SyncPlay { // Client, that was buffering, resumed playback but did not update others in time. delayTicks = context.GetHighestPing() * 2 * TimeSpan.TicksPerMillisecond; - delayTicks = delayTicks < context.DefaultPing ? context.DefaultPing : delayTicks; + delayTicks = Math.Max(delayTicks, context.DefaultPing); context.LastActivity = currentTime.AddTicks(delayTicks); diff --git a/MediaBrowser.Controller/SyncPlay/Queue/PlayQueueManager.cs b/MediaBrowser.Controller/SyncPlay/Queue/PlayQueueManager.cs index 366bedfa1..9ab0dfd2a 100644 --- a/MediaBrowser.Controller/SyncPlay/Queue/PlayQueueManager.cs +++ b/MediaBrowser.Controller/SyncPlay/Queue/PlayQueueManager.cs @@ -25,7 +25,7 @@ namespace MediaBrowser.Controller.SyncPlay /// /// Class PlayQueueManager. /// - public class PlayQueueManager : IDisposable + public class PlayQueueManager { /// /// Gets or sets the playing item index. @@ -83,27 +83,6 @@ namespace MediaBrowser.Controller.SyncPlay Reset(); } - /// - public void Dispose() - { - Dispose(true); - GC.SuppressFinalize(this); - } - - /// - /// Releases unmanaged and optionally managed resources. - /// - /// true to release both managed and unmanaged resources; false to release only unmanaged resources. - protected virtual void Dispose(bool disposing) - { - if (_disposed) - { - return; - } - - _disposed = true; - } - /// /// Gets the next available identifier. /// @@ -284,7 +263,7 @@ namespace MediaBrowser.Controller.SyncPlay ShuffledPlaylist.Add(playingItem); } PlayingItemIndex = 0; - } + } else { PlayingItemIndex = NoPlayingItemIndex;