Refactor and other minor changes

Emby.Server.Implementations/Syncplay/SyncplayController.cs

@@ -16,11 +16,26 @@ namespace Emby.Server.Implementations.Syncplay
     /// </summary>
     public class SyncplayController : ISyncplayController, IDisposable
     {
+        /// <summary>
+        /// Used to filter the sessions of a group.
+        /// </summary>
         private enum BroadcastType
         {
+            /// <summary>
+            /// All sessions will receive the message.
+            /// </summary>
             AllGroup = 0,
-            SingleSession = 1,
-            AllExceptSession = 2,
+            /// <summary>
+            /// Only the specified session will receive the message.
+            /// </summary>
+            CurrentSession = 1,
+            /// <summary>
+            /// All sessions, except the current one, will receive the message.
+            /// </summary>
+            AllExceptCurrentSession = 2,
+            /// <summary>
+            /// Only sessions that are not buffering will receive the message.
+            /// </summary>
             AllReady = 3
         }
 
@@ -95,40 +110,46 @@ namespace Emby.Server.Implementations.Syncplay
             }
         }
 
+        /// <summary>
+        /// Filters sessions of this group.
+        /// </summary>
+        /// <param name="from">The current session.</param>
+        /// <param name="type">The filtering type.</param>
+        /// <value>The array of sessions matching the filter.</value>
         private SessionInfo[] FilterSessions(SessionInfo from, BroadcastType type)
         {
-            if (type == BroadcastType.SingleSession)
+            switch (type)
             {
-                return new SessionInfo[] { from };
-            }
-            else if (type == BroadcastType.AllGroup)
-            {
-                return _group.Partecipants.Values.Select(
-                    session => session.Session
-                ).ToArray();
-            }
-            else if (type == BroadcastType.AllExceptSession)
-            {
-                return _group.Partecipants.Values.Select(
-                    session => session.Session
-                ).Where(
-                    session => !session.Id.Equals(from.Id)
-                ).ToArray();
-            }
-            else if (type == BroadcastType.AllReady)
-            {
-                return _group.Partecipants.Values.Where(
-                    session => !session.IsBuffering
-                ).Select(
-                    session => session.Session
-                ).ToArray();
-            }
-            else
-            {
-                return new SessionInfo[] {};
+                case BroadcastType.CurrentSession:
+                    return new SessionInfo[] { from };
+                case BroadcastType.AllGroup:
+                    return _group.Participants.Values.Select(
+                        session => session.Session
+                    ).ToArray();
+                case BroadcastType.AllExceptCurrentSession:
+                    return _group.Participants.Values.Select(
+                        session => session.Session
+                    ).Where(
+                        session => !session.Id.Equals(from.Id)
+                    ).ToArray();
+                case BroadcastType.AllReady:
+                    return _group.Participants.Values.Where(
+                        session => !session.IsBuffering
+                    ).Select(
+                        session => session.Session
+                    ).ToArray();
+                default:
+                    return new SessionInfo[] { };
             }
         }
 
+        /// <summary>
+        /// Sends a GroupUpdate message to the interested sessions.
+        /// </summary>
+        /// <param name="from">The current session.</param>
+        /// <param name="type">The filtering type.</param>
+        /// <param name="message">The message to send.</param>
+        /// <value>The task.</value>
         private Task SendGroupUpdate<T>(SessionInfo from, BroadcastType type, GroupUpdate<T> message)
         {
             IEnumerable<Task> GetTasks()
@@ -143,6 +164,13 @@ namespace Emby.Server.Implementations.Syncplay
             return Task.WhenAll(GetTasks());
         }
 
+        /// <summary>
+        /// Sends a playback command to the interested sessions.
+        /// </summary>
+        /// <param name="from">The current session.</param>
+        /// <param name="type">The filtering type.</param>
+        /// <param name="message">The message to send.</param>
+        /// <value>The task.</value>
         private Task SendCommand(SessionInfo from, BroadcastType type, SendCommand message)
         {
             IEnumerable<Task> GetTasks()
@@ -157,31 +185,44 @@ namespace Emby.Server.Implementations.Syncplay
             return Task.WhenAll(GetTasks());
         }
 
+        /// <summary>
+        /// Builds a new playback command with some default values.
+        /// </summary>
+        /// <param name="type">The command type.</param>
+        /// <value>The SendCommand.</value>
         private SendCommand NewSyncplayCommand(SendCommandType type)
         {
-            var command = new SendCommand();
-            command.GroupId = _group.GroupId.ToString();
-            command.Command = type;
-            command.PositionTicks = _group.PositionTicks;
-            command.When = _group.LastActivity.ToUniversalTime().ToString("o");
-            command.EmittedAt = DateTime.UtcNow.ToUniversalTime().ToString("o");
-            return command;
+            return new SendCommand()
+            {
+                GroupId = _group.GroupId.ToString(),
+                Command = type,
+                PositionTicks = _group.PositionTicks,
+                When = _group.LastActivity.ToUniversalTime().ToString("o"),
+                EmittedAt = DateTime.UtcNow.ToUniversalTime().ToString("o")
+            };
         }
 
+        /// <summary>
+        /// Builds a new group update message.
+        /// </summary>
+        /// <param name="type">The update type.</param>
+        /// <param name="data">The data to send.</param>
+        /// <value>The GroupUpdate.</value>
         private GroupUpdate<T> NewSyncplayGroupUpdate<T>(GroupUpdateType type, T data)
         {
-            var command = new GroupUpdate<T>();
-            command.GroupId = _group.GroupId.ToString();
-            command.Type = type;
-            command.Data = data;
-            return command;
+            return new GroupUpdate<T>()
+            {
+                GroupId = _group.GroupId.ToString(),
+                Type = type,
+                Data = data
+            };
         }
 
         /// <inheritdoc />
         public void InitGroup(SessionInfo session)
         {
             _group.AddSession(session);
-            _syncplayManager.MapSessionToGroup(session, this);
+            _syncplayManager.AddSessionToGroup(session, this);
 
             _group.PlayingItem = session.FullNowPlayingItem;
             _group.IsPaused = true;
@@ -189,37 +230,35 @@ namespace Emby.Server.Implementations.Syncplay
             _group.LastActivity = DateTime.UtcNow;
 
             var updateSession = NewSyncplayGroupUpdate(GroupUpdateType.GroupJoined, DateTime.UtcNow.ToUniversalTime().ToString("o"));
-            SendGroupUpdate(session, BroadcastType.SingleSession, updateSession);
+            SendGroupUpdate(session, BroadcastType.CurrentSession, updateSession);
             var pauseCommand = NewSyncplayCommand(SendCommandType.Pause);
-            SendCommand(session, BroadcastType.SingleSession, pauseCommand);
+            SendCommand(session, BroadcastType.CurrentSession, pauseCommand);
         }
 
         /// <inheritdoc />
         public void SessionJoin(SessionInfo session, JoinGroupRequest request)
         {
-            if (session.NowPlayingItem != null &&
-                session.NowPlayingItem.Id.Equals(_group.PlayingItem.Id) &&
-                request.PlayingItemId.Equals(_group.PlayingItem.Id))
+            if (session.NowPlayingItem?.Id == _group.PlayingItem.Id && request.PlayingItemId == _group.PlayingItem.Id)
             {
                 _group.AddSession(session);
-                _syncplayManager.MapSessionToGroup(session, this);
+                _syncplayManager.AddSessionToGroup(session, this);
 
                 var updateSession = NewSyncplayGroupUpdate(GroupUpdateType.GroupJoined, DateTime.UtcNow.ToUniversalTime().ToString("o"));
-                SendGroupUpdate(session, BroadcastType.SingleSession, updateSession);
+                SendGroupUpdate(session, BroadcastType.CurrentSession, updateSession);
 
                 var updateOthers = NewSyncplayGroupUpdate(GroupUpdateType.UserJoined, session.UserName);
-                SendGroupUpdate(session, BroadcastType.AllExceptSession, updateOthers);
+                SendGroupUpdate(session, BroadcastType.AllExceptCurrentSession, updateOthers);
 
                 // Client join and play, syncing will happen client side
                 if (!_group.IsPaused)
                 {
                     var playCommand = NewSyncplayCommand(SendCommandType.Play);
-                    SendCommand(session, BroadcastType.SingleSession, playCommand);
+                    SendCommand(session, BroadcastType.CurrentSession, playCommand);
                 }
                 else
                 {
                     var pauseCommand = NewSyncplayCommand(SendCommandType.Pause);
-                    SendCommand(session, BroadcastType.SingleSession, pauseCommand);
+                    SendCommand(session, BroadcastType.CurrentSession, pauseCommand);
                 }
             }
             else
@@ -228,7 +267,7 @@ namespace Emby.Server.Implementations.Syncplay
                 playRequest.ItemIds = new Guid[] { _group.PlayingItem.Id };
                 playRequest.StartPositionTicks = _group.PositionTicks;
                 var update = NewSyncplayGroupUpdate(GroupUpdateType.PrepareSession, playRequest);
-                SendGroupUpdate(session, BroadcastType.SingleSession, update);
+                SendGroupUpdate(session, BroadcastType.CurrentSession, update);
             }
         }
 
@@ -236,182 +275,250 @@ namespace Emby.Server.Implementations.Syncplay
         public void SessionLeave(SessionInfo session)
         {
             _group.RemoveSession(session);
-            _syncplayManager.UnmapSessionFromGroup(session, this);
+            _syncplayManager.RemoveSessionFromGroup(session, this);
 
             var updateSession = NewSyncplayGroupUpdate(GroupUpdateType.GroupLeft, _group.PositionTicks);
-            SendGroupUpdate(session, BroadcastType.SingleSession, updateSession);
+            SendGroupUpdate(session, BroadcastType.CurrentSession, updateSession);
 
             var updateOthers = NewSyncplayGroupUpdate(GroupUpdateType.UserLeft, session.UserName);
-            SendGroupUpdate(session, BroadcastType.AllExceptSession, updateOthers);
+            SendGroupUpdate(session, BroadcastType.AllExceptCurrentSession, updateOthers);
         }
 
         /// <inheritdoc />
         public void HandleRequest(SessionInfo session, PlaybackRequest request)
         {
-            if (request.Type.Equals(PlaybackRequestType.Play))
+            // The server's job is to mantain a consistent state to which clients refer to,
+            // as also to notify clients of state changes.
+            // The actual syncing of media playback happens client side.
+            // Clients are aware of the server's time and use it to sync.
+            switch (request.Type)
             {
-                if (_group.IsPaused)
-                {
-                    var delay = _group.GetHighestPing() * 2;
-                    delay = delay < _group.DefaulPing ? _group.DefaulPing : delay;
-
-                    _group.IsPaused = false;
-                    _group.LastActivity = DateTime.UtcNow.AddMilliseconds(
-                        delay
-                    );
-
-                    var command = NewSyncplayCommand(SendCommandType.Play);
-                    SendCommand(session, BroadcastType.AllGroup, command);
-                }
-                else
-                {
-                    // Client got lost
-                    var command = NewSyncplayCommand(SendCommandType.Play);
-                    SendCommand(session, BroadcastType.SingleSession, command);
-                }
+                case PlaybackRequestType.Play:
+                    HandlePlayRequest(session, request);
+                    break;
+                case PlaybackRequestType.Pause:
+                    HandlePauseRequest(session, request);
+                    break;
+                case PlaybackRequestType.Seek:
+                    HandleSeekRequest(session, request);
+                    break;
+                case PlaybackRequestType.Buffering:
+                    HandleBufferingRequest(session, request);
+                    break;
+                case PlaybackRequestType.BufferingDone:
+                    HandleBufferingDoneRequest(session, request);
+                    break;
+                case PlaybackRequestType.UpdatePing:
+                    HandlePingUpdateRequest(session, request);
+                    break;
             }
-            else if (request.Type.Equals(PlaybackRequestType.Pause))
+        }
+
+        /// <summary>
+        /// Handles a play action requested by a session.
+        /// </summary>
+        /// <param name="session">The session.</param>
+        /// <param name="request">The play action.</param>
+        private void HandlePlayRequest(SessionInfo session, PlaybackRequest request)
+        {
+            if (_group.IsPaused)
             {
-                if (!_group.IsPaused)
-                {
-                    _group.IsPaused = true;
-                    var currentTime = DateTime.UtcNow;
-                    var elapsedTime = currentTime - _group.LastActivity;
-                    _group.LastActivity = currentTime;
-                    _group.PositionTicks += elapsedTime.Ticks > 0 ? elapsedTime.Ticks : 0;
+                // Pick a suitable time that accounts for latency
+                var delay = _group.GetHighestPing() * 2;
+                delay = delay < _group.DefaulPing ? _group.DefaulPing : delay;
 
-                    var command = NewSyncplayCommand(SendCommandType.Pause);
-                    SendCommand(session, BroadcastType.AllGroup, command);
-                }
-                else
-                {
-                    var command = NewSyncplayCommand(SendCommandType.Pause);
-                    SendCommand(session, BroadcastType.SingleSession, command);
-                }
-            }
-            else if (request.Type.Equals(PlaybackRequestType.Seek))
-            {
-                // Sanitize PositionTicks
-                var ticks = request.PositionTicks ??= 0;
-                ticks = ticks >= 0 ? ticks : 0;
-                if (_group.PlayingItem.RunTimeTicks != null)
-                {
-                    var runTimeTicks = _group.PlayingItem.RunTimeTicks ??= 0;
-                    ticks = ticks > runTimeTicks ? runTimeTicks : ticks;
-                }
+                // Unpause group and set starting point in future
+                // Clients will start playback at LastActivity (datetime) from PositionTicks (playback position)
+                // The added delay does not guarantee, of course, that the command will be received in time
+                // Playback synchronization will mainly happen client side
+                _group.IsPaused = false;
+                _group.LastActivity = DateTime.UtcNow.AddMilliseconds(
+                    delay
+                );
 
-                _group.IsPaused = true;
-                _group.PositionTicks = ticks;
-                _group.LastActivity = DateTime.UtcNow;
-
-                var command = NewSyncplayCommand(SendCommandType.Seek);
+                var command = NewSyncplayCommand(SendCommandType.Play);
                 SendCommand(session, BroadcastType.AllGroup, command);
             }
-            // TODO: client does not implement this yet
-            else if (request.Type.Equals(PlaybackRequestType.Buffering))
+            else
             {
-                if (!_group.IsPaused)
+                // Client got lost, sending current state
+                var command = NewSyncplayCommand(SendCommandType.Play);
+                SendCommand(session, BroadcastType.CurrentSession, command);
+            }
+        }
+
+        /// <summary>
+        /// Handles a pause action requested by a session.
+        /// </summary>
+        /// <param name="session">The session.</param>
+        /// <param name="request">The pause action.</param>
+        private void HandlePauseRequest(SessionInfo session, PlaybackRequest request)
+        {
+            if (!_group.IsPaused)
+            {
+                // Pause group and compute the media playback position
+                _group.IsPaused = true;
+                var currentTime = DateTime.UtcNow;
+                var elapsedTime = currentTime - _group.LastActivity;
+                _group.LastActivity = currentTime;
+                // Seek only if playback actually started
+                // (a pause request may be issued during the delay added to account for latency)
+                _group.PositionTicks += elapsedTime.Ticks > 0 ? elapsedTime.Ticks : 0;
+
+                var command = NewSyncplayCommand(SendCommandType.Pause);
+                SendCommand(session, BroadcastType.AllGroup, command);
+            }
+            else
+            {
+                // Client got lost, sending current state
+                var command = NewSyncplayCommand(SendCommandType.Pause);
+                SendCommand(session, BroadcastType.CurrentSession, command);
+            }
+        }
+
+        /// <summary>
+        /// Handles a seek action requested by a session.
+        /// </summary>
+        /// <param name="session">The session.</param>
+        /// <param name="request">The seek action.</param>
+        private void HandleSeekRequest(SessionInfo session, PlaybackRequest request)
+        {
+            // Sanitize PositionTicks
+            var ticks = request.PositionTicks ??= 0;
+            ticks = ticks >= 0 ? ticks : 0;
+            if (_group.PlayingItem.RunTimeTicks != null)
+            {
+                var runTimeTicks = _group.PlayingItem.RunTimeTicks ??= 0;
+                ticks = ticks > runTimeTicks ? runTimeTicks : ticks;
+            }
+
+            // Pause and seek
+            _group.IsPaused = true;
+            _group.PositionTicks = ticks;
+            _group.LastActivity = DateTime.UtcNow;
+
+            var command = NewSyncplayCommand(SendCommandType.Seek);
+            SendCommand(session, BroadcastType.AllGroup, command);
+        }
+
+        /// <summary>
+        /// Handles a buffering action requested by a session.
+        /// </summary>
+        /// <param name="session">The session.</param>
+        /// <param name="request">The buffering action.</param>
+        private void HandleBufferingRequest(SessionInfo session, PlaybackRequest request)
+        {
+            if (!_group.IsPaused)
+            {
+                // Pause group and compute the media playback position
+                _group.IsPaused = true;
+                var currentTime = DateTime.UtcNow;
+                var elapsedTime = currentTime - _group.LastActivity;
+                _group.LastActivity = currentTime;
+                _group.PositionTicks += elapsedTime.Ticks > 0 ? elapsedTime.Ticks : 0;
+
+                _group.SetBuffering(session, true);
+
+                // Send pause command to all non-buffering sessions
+                var command = NewSyncplayCommand(SendCommandType.Pause);
+                SendCommand(session, BroadcastType.AllReady, command);
+
+                var updateOthers = NewSyncplayGroupUpdate(GroupUpdateType.GroupWait, session.UserName);
+                SendGroupUpdate(session, BroadcastType.AllExceptCurrentSession, updateOthers);
+            }
+            else
+            {
+                // Client got lost, sending current state
+                var command = NewSyncplayCommand(SendCommandType.Pause);
+                SendCommand(session, BroadcastType.CurrentSession, command);
+            }
+        }
+
+        /// <summary>
+        /// Handles a buffering-done action requested by a session.
+        /// </summary>
+        /// <param name="session">The session.</param>
+        /// <param name="request">The buffering-done action.</param>
+        private void HandleBufferingDoneRequest(SessionInfo session, PlaybackRequest request)
+        {
+            if (_group.IsPaused)
+            {
+                _group.SetBuffering(session, false);
+
+                var when = request.When ??= DateTime.UtcNow;
+                var currentTime = DateTime.UtcNow;
+                var elapsedTime = currentTime - when;
+                var clientPosition = TimeSpan.FromTicks(request.PositionTicks ??= 0) + elapsedTime;
+                var delay = _group.PositionTicks - clientPosition.Ticks;
+
+                if (_group.IsBuffering())
                 {
-                    _group.IsPaused = true;
-                    var currentTime = DateTime.UtcNow;
-                    var elapsedTime = currentTime - _group.LastActivity;
-                    _group.LastActivity = currentTime;
-                    _group.PositionTicks += elapsedTime.Ticks > 0 ? elapsedTime.Ticks : 0;
-
-                    _group.SetBuffering(session, true);
-
-                    // Send pause command to all non-buffering sessions
+                    // Others are buffering, tell this client to pause when ready
                     var command = NewSyncplayCommand(SendCommandType.Pause);
-                    SendCommand(session, BroadcastType.AllReady, command);
-
-                    var updateOthers = NewSyncplayGroupUpdate(GroupUpdateType.GroupWait, session.UserName);
-                    SendGroupUpdate(session, BroadcastType.AllExceptSession, updateOthers);
+                    command.When = currentTime.AddMilliseconds(
+                        delay
+                    ).ToUniversalTime().ToString("o");
+                    SendCommand(session, BroadcastType.CurrentSession, command);
                 }
                 else
                 {
-                    var command = NewSyncplayCommand(SendCommandType.Pause);
-                    SendCommand(session, BroadcastType.SingleSession, command);
-                }
-            }
-            // TODO: client does not implement this yet
-            else if (request.Type.Equals(PlaybackRequestType.BufferingComplete))
-            {
-                if (_group.IsPaused)
-                {
-                    _group.SetBuffering(session, false);
+                    // Let other clients resume as soon as the buffering client catches up
+                    _group.IsPaused = false;
 
-                    if (_group.IsBuffering()) {
-                        // Others are buffering, tell this client to pause when ready
-                        var when = request.When ??= DateTime.UtcNow;
-                        var currentTime = DateTime.UtcNow;
-                        var elapsedTime = currentTime - when;
-                        var clientPosition = TimeSpan.FromTicks(request.PositionTicks ??= 0) + elapsedTime;
-                        var delay = _group.PositionTicks - clientPosition.Ticks;
-
-                        var command = NewSyncplayCommand(SendCommandType.Pause);
-                        command.When = currentTime.AddMilliseconds(
+                    if (delay > _group.GetHighestPing() * 2)
+                    {
+                        // Client that was buffering is recovering, notifying others to resume
+                        _group.LastActivity = currentTime.AddMilliseconds(
                             delay
-                        ).ToUniversalTime().ToString("o");
-                        SendCommand(session, BroadcastType.SingleSession, command);
+                        );
+                        var command = NewSyncplayCommand(SendCommandType.Play);
+                        SendCommand(session, BroadcastType.AllExceptCurrentSession, command);
                     }
                     else
                     {
-                        // Let other clients resume as soon as the buffering client catches up
-                        var when = request.When ??= DateTime.UtcNow;
-                        var currentTime = DateTime.UtcNow;
-                        var elapsedTime = currentTime - when;
-                        var clientPosition = TimeSpan.FromTicks(request.PositionTicks ??= 0) + elapsedTime;
-                        var delay = _group.PositionTicks - clientPosition.Ticks;
+                        // Client, that was buffering, resumed playback but did not update others in time
+                        delay = _group.GetHighestPing() * 2;
+                        delay = delay < _group.DefaulPing ? _group.DefaulPing : delay;
 
-                        _group.IsPaused = false;
+                        _group.LastActivity = currentTime.AddMilliseconds(
+                            delay
+                        );
 
-                        if (delay > _group.GetHighestPing() * 2)
-                        {
-                            // Client that was buffering is recovering, notifying others to resume
-                            _group.LastActivity = currentTime.AddMilliseconds(
-                                delay
-                            );
-                            var command = NewSyncplayCommand(SendCommandType.Play);
-                            SendCommand(session, BroadcastType.AllExceptSession, command);
-                        }
-                        else
-                        {
-                            // Client, that was buffering, resumed playback but did not update others in time
-                            delay = _group.GetHighestPing() * 2;
-                            delay = delay < _group.DefaulPing ? _group.DefaulPing : delay;
-
-                            _group.LastActivity = currentTime.AddMilliseconds(
-                                delay
-                            );
-
-                            var command = NewSyncplayCommand(SendCommandType.Play);
-                            SendCommand(session, BroadcastType.AllGroup, command);
-                        }
-                    }                    
-                }
-                else
-                {
-                    // Make sure client has latest group state
-                    var command = NewSyncplayCommand(SendCommandType.Play);
-                    SendCommand(session, BroadcastType.SingleSession, command);
+                        var command = NewSyncplayCommand(SendCommandType.Play);
+                        SendCommand(session, BroadcastType.AllGroup, command);
+                    }
                 }
             }
-            else if (request.Type.Equals(PlaybackRequestType.UpdatePing))
+            else
             {
-                _group.UpdatePing(session, request.Ping ??= _group.DefaulPing);
+                // Group was not waiting, make sure client has latest state
+                var command = NewSyncplayCommand(SendCommandType.Play);
+                SendCommand(session, BroadcastType.CurrentSession, command);
             }
         }
 
+        /// <summary>
+        /// Updates ping of a session.
+        /// </summary>
+        /// <param name="session">The session.</param>
+        /// <param name="request">The update.</param>
+        private void HandlePingUpdateRequest(SessionInfo session, PlaybackRequest request)
+        {
+            // Collected pings are used to account for network latency when unpausing playback
+            _group.UpdatePing(session, request.Ping ??= _group.DefaulPing);
+        }
+
         /// <inheritdoc />
         public GroupInfoView GetInfo()
         {
-            var info = new GroupInfoView();
-            info.GroupId = GetGroupId().ToString();
-            info.PlayingItemName = _group.PlayingItem.Name;
-            info.PlayingItemId = _group.PlayingItem.Id.ToString();
-            info.PositionTicks = _group.PositionTicks;
-            info.Partecipants = _group.Partecipants.Values.Select(session => session.Session.UserName).ToArray();
-            return info;
+            return new GroupInfoView()
+            {
+                GroupId = GetGroupId().ToString(),
+                PlayingItemName = _group.PlayingItem.Name,
+                PlayingItemId = _group.PlayingItem.Id.ToString(),
+                PositionTicks = _group.PositionTicks,
+                Participants = _group.Participants.Values.Select(session => session.Session.UserName).ToArray()
+            };
         }
     }
 }

Emby.Server.Implementations/Syncplay/SyncplayManager.cs

@@ -49,7 +49,7 @@ namespace Emby.Server.Implementations.Syncplay
         /// The groups.
         /// </summary>
         private readonly ConcurrentDictionary<string, ISyncplayController> _groups =
-        new ConcurrentDictionary<string, ISyncplayController>(StringComparer.OrdinalIgnoreCase);
+            new ConcurrentDictionary<string, ISyncplayController>(StringComparer.OrdinalIgnoreCase);
 
         private bool _disposed = false;
 
@@ -64,8 +64,8 @@ namespace Emby.Server.Implementations.Syncplay
             _sessionManager = sessionManager;
             _libraryManager = libraryManager;
 
-            _sessionManager.SessionEnded += _sessionManager_SessionEnded;
-            _sessionManager.PlaybackStopped += _sessionManager_PlaybackStopped;
+            _sessionManager.SessionEnded += OnSessionManagerSessionEnded;
+            _sessionManager.PlaybackStopped += OnSessionManagerPlaybackStopped;
         }
 
         /// <summary>
@@ -92,8 +92,8 @@ namespace Emby.Server.Implementations.Syncplay
                 return;
             }
 
-            _sessionManager.SessionEnded -= _sessionManager_SessionEnded;
-            _sessionManager.PlaybackStopped -= _sessionManager_PlaybackStopped;
+            _sessionManager.SessionEnded -= OnSessionManagerSessionEnded;
+            _sessionManager.PlaybackStopped -= OnSessionManagerPlaybackStopped;
 
             _disposed = true;
         }
@@ -106,14 +106,14 @@ namespace Emby.Server.Implementations.Syncplay
             }
         }
 
-        void _sessionManager_SessionEnded(object sender, SessionEventArgs e)
+        private void OnSessionManagerSessionEnded(object sender, SessionEventArgs e)
         {
             var session = e.SessionInfo;
             if (!IsSessionInGroup(session)) return;
             LeaveGroup(session);
         }
 
-        void _sessionManager_PlaybackStopped(object sender, PlaybackStopEventArgs e)
+        private void OnSessionManagerPlaybackStopped(object sender, PlaybackStopEventArgs e)
         {
             var session = e.Session;
             if (!IsSessionInGroup(session)) return;
@@ -130,13 +130,13 @@ namespace Emby.Server.Implementations.Syncplay
             var item = _libraryManager.GetItemById(itemId);
             var hasParentalRatingAccess = user.Policy.MaxParentalRating.HasValue ? item.InheritedParentalRatingValue <= user.Policy.MaxParentalRating : true;
 
-            if (!user.Policy.EnableAllFolders)
+            if (!user.Policy.EnableAllFolders && hasParentalRatingAccess)
             {
                 var collections = _libraryManager.GetCollectionFolders(item).Select(
                     folder => folder.Id.ToString("N", CultureInfo.InvariantCulture)
                 );
                 var intersect = collections.Intersect(user.Policy.EnabledFolders);
-                return intersect.Count() > 0 && hasParentalRatingAccess;
+                return intersect.Count() > 0;
             }
             else
             {
@@ -165,7 +165,7 @@ namespace Emby.Server.Implementations.Syncplay
 
             if (user.Policy.SyncplayAccess != SyncplayAccess.CreateAndJoinGroups)
             {
-                // TODO: shall an error message be sent back to the client?
+                // TODO: report the error to the client
                 throw new ArgumentException("User does not have permission to create groups");
             }
 
@@ -187,22 +187,16 @@ namespace Emby.Server.Implementations.Syncplay
 
             if (user.Policy.SyncplayAccess == SyncplayAccess.None)
             {
-                // TODO: shall an error message be sent back to the client?
+                // TODO: report the error to the client
                 throw new ArgumentException("User does not have access to syncplay");
             }
 
-            if (IsSessionInGroup(session))
-            {
-                if (GetSessionGroup(session).Equals(groupId)) return;
-                LeaveGroup(session);
-            }
-
             ISyncplayController group;
             _groups.TryGetValue(groupId, out group);
 
             if (group == null)
             {
-                _logger.LogError("Syncplaymanager JoinGroup: " + groupId + " does not exist.");
+                _logger.LogWarning("Syncplaymanager JoinGroup: {0} does not exist.", groupId);
 
                 var update = new GroupUpdate<string>();
                 update.Type = GroupUpdateType.NotInGroup;
@@ -215,20 +209,26 @@ namespace Emby.Server.Implementations.Syncplay
                 throw new ArgumentException("User does not have access to playing item");
             }
 
+            if (IsSessionInGroup(session))
+            {
+                if (GetSessionGroup(session).Equals(groupId)) return;
+                LeaveGroup(session);
+            }
+
             group.SessionJoin(session, request);
         }
 
         /// <inheritdoc />
         public void LeaveGroup(SessionInfo session)
         {
-            // TODO: what happens to users that are in a group and get their permissions revoked?
+            // TODO: determine what happens to users that are in a group and get their permissions revoked
 
             ISyncplayController group;
             _sessionToGroupMap.TryGetValue(session.Id, out group);
 
             if (group == null)
             {
-                _logger.LogWarning("Syncplaymanager HandleRequest: " + session.Id + " not in group.");
+                _logger.LogWarning("Syncplaymanager LeaveGroup: {0} does not belong to any group.", session.Id);
 
                 var update = new GroupUpdate<string>();
                 update.Type = GroupUpdateType.NotInGroup;
@@ -257,9 +257,7 @@ namespace Emby.Server.Implementations.Syncplay
             if (session.NowPlayingItem != null)
             {
                 return _groups.Values.Where(
-                    group => HasAccessToItem(user, group.GetPlayingItemId())
-                ).Where(
-                    group => group.GetPlayingItemId().Equals(session.FullNowPlayingItem.Id)
+                    group => group.GetPlayingItemId().Equals(session.FullNowPlayingItem.Id) && HasAccessToItem(user, group.GetPlayingItemId())
                 ).Select(
                     group => group.GetInfo()
                 ).ToList();
@@ -291,7 +289,7 @@ namespace Emby.Server.Implementations.Syncplay
 
             if (group == null)
             {
-                _logger.LogWarning("Syncplaymanager HandleRequest: " + session.Id + " not in group.");
+                _logger.LogWarning("Syncplaymanager HandleRequest: {0} not in a group.", session.Id);
 
                 var update = new GroupUpdate<string>();
                 update.Type = GroupUpdateType.NotInGroup;
@@ -302,7 +300,7 @@ namespace Emby.Server.Implementations.Syncplay
         }
 
         /// <inheritdoc />
-        public void MapSessionToGroup(SessionInfo session, ISyncplayController group)
+        public void AddSessionToGroup(SessionInfo session, ISyncplayController group)
         {
             if (IsSessionInGroup(session))
             {
@@ -312,7 +310,7 @@ namespace Emby.Server.Implementations.Syncplay
         }
 
         /// <inheritdoc />
-        public void UnmapSessionFromGroup(SessionInfo session, ISyncplayController group)
+        public void RemoveSessionFromGroup(SessionInfo session, ISyncplayController group)
         {
             if (!IsSessionInGroup(session))
             {