From e9d25a69a06d46c779d50e13b29ae993137ad25b Mon Sep 17 00:00:00 2001 From: Peter Marheine Date: Thu, 15 Oct 2015 17:20:22 -0600 Subject: [PATCH 1/6] Add status query cookie. This helps prevent fighting when the player takes a long time to report changes in status. It currently fails to play/pause in response to remote commands when latency is sufficiently high, however. --- syncplay/client.py | 35 ++++++++++++++++++++++++++--------- 1 file changed, 26 insertions(+), 9 deletions(-) diff --git a/syncplay/client.py b/syncplay/client.py index 3d135e9..7de9738 100644 --- a/syncplay/client.py +++ b/syncplay/client.py @@ -110,6 +110,8 @@ class SyncplayClient(object): self.autoPlay = False self.autoPlayThreshold = None + self._lastPlayerCommand = time.time() + self.autoplayTimer = task.LoopingCall(self.autoplayCountdown) self.autoplayTimeLeft = constants.AUTOPLAY_DELAY @@ -137,6 +139,16 @@ class SyncplayClient(object): constants.OSD_WARNING_MESSAGE_DURATION = constants.NO_SECONDARY_OSD_WARNING_DURATION self.scheduleAskPlayer() + def _playerRequest(self, f, *args, **kwargs): + """Send a request with cookie to the player.""" + kwargs['cookie'] = time.time() + f(*args, **kwargs) + + def _playerCommand(self, f, *args, **kwargs): + """Send a command to the player, affecting cookie freshness.""" + self._lastPlayerCommand = time.time() + f(*args, **kwargs) + def scheduleAskPlayer(self, when=constants.PLAYER_ASK_DELAY): self._askPlayerTimer = task.LoopingCall(self.askPlayer) self._askPlayerTimer.start(when) @@ -145,7 +157,7 @@ class SyncplayClient(object): if not self._running: return if self._player: - self._player.askForStatus() + self._playerRequest(self._player.askForStatus) self.checkIfConnected() def checkIfConnected(self): @@ -163,14 +175,19 @@ class SyncplayClient(object): seeked = _playerDiff > constants.SEEK_THRESHOLD and _globalDiff > constants.SEEK_THRESHOLD return pauseChange, seeked - def updatePlayerStatus(self, paused, position): + def updatePlayerStatus(self, paused, position, cookie=None): + # Ignore status report if the cookie is stale + if cookie is not None and \ + cookie < constants.PLAYER_COMMAND_DELAY + self._lastPlayerCommand: + return + position -= self.getUserOffset() pauseChange, seeked = self._determinePlayerStateChange(paused, position) self._playerPosition = position self._playerPaused = paused if pauseChange and utils.meetsMinVersion(self.serverVersion, constants.USER_READY_MIN_VERSION): if not self.userlist.currentUser.canControl(): - self._player.setPaused(self._globalPaused) + self._playerCommand(self._player.setPaused, self._globalPaused) self.toggleReady(manuallyInitiated=True) self._playerPaused = self._globalPaused pauseChange = False @@ -180,7 +197,7 @@ class SyncplayClient(object): self.ui.showMessage(getMessage("set-as-not-ready-notification")) elif not paused and not self.instaplayConditionsMet(): paused = True - self._player.setPaused(paused) + self._playerCommand(self._player.setPaused, paused) self._playerPaused = paused self.changeReadyState(True, manuallyInitiated=True) pauseChange = False @@ -213,7 +230,7 @@ class SyncplayClient(object): def _initPlayerState(self, position, paused): if self.userlist.currentUser.file: self.setPosition(position) - self._player.setPaused(paused) + self._playerCommand(self._player.setPaused, paused) madeChangeOnPlayer = True return madeChangeOnPlayer @@ -241,7 +258,7 @@ class SyncplayClient(object): def _serverUnpaused(self, setBy): hideFromOSD = not constants.SHOW_SAME_ROOM_OSD - self._player.setPaused(False) + self._playerCommand(self._player.setPaused, False) madeChangeOnPlayer = True self.ui.showMessage(getMessage("unpause-notification").format(setBy), hideFromOSD) return madeChangeOnPlayer @@ -250,7 +267,7 @@ class SyncplayClient(object): hideFromOSD = not constants.SHOW_SAME_ROOM_OSD if constants.SYNC_ON_PAUSE and self.getUsername() <> setBy: self.setPosition(self.getGlobalPosition()) - self._player.setPaused(True) + self._playerCommand(self._player.setPaused, True) madeChangeOnPlayer = True if (self.lastLeftTime < time.time() - constants.OSD_DURATION) or (hideFromOSD == True): self.ui.showMessage(getMessage("pause-notification").format(setBy), hideFromOSD) @@ -498,13 +515,13 @@ class SyncplayClient(object): if position < 0: position = 0 self._protocol.sendState(self.getPlayerPosition(), self.getPlayerPaused(), True, None, True) - self._player.setPosition(position) + self._playerCommand(self._player.setPosition, position) def setPaused(self, paused): if self._player and self.userlist.currentUser.file: if self._lastPlayerUpdate and not paused: self._lastPlayerUpdate = time.time() - self._player.setPaused(paused) + self._playerCommand(self._player.setPaused, paused) def start(self, host, port): if self._running: From d2ce799bfec5c19743692f0d87d84e53505e41f1 Mon Sep 17 00:00:00 2001 From: Peter Marheine Date: Thu, 15 Oct 2015 17:35:05 -0600 Subject: [PATCH 2/6] Immediately update local player state on server change If status replies are ignored, the result of a server play/pause command could be ignored as well (since the old playing status would end up being seen as a local restart), so set it immediately on dispatching the player command. --- syncplay/client.py | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/syncplay/client.py b/syncplay/client.py index 7de9738..22da494 100644 --- a/syncplay/client.py +++ b/syncplay/client.py @@ -258,6 +258,10 @@ class SyncplayClient(object): def _serverUnpaused(self, setBy): hideFromOSD = not constants.SHOW_SAME_ROOM_OSD + # In high-player-latency situations we might report our state back to + # the server before any player status is accepted as fresh. Override + # the locally-stored playback state. + self._playerPaused = False self._playerCommand(self._player.setPaused, False) madeChangeOnPlayer = True self.ui.showMessage(getMessage("unpause-notification").format(setBy), hideFromOSD) @@ -265,6 +269,7 @@ class SyncplayClient(object): def _serverPaused(self, setBy): hideFromOSD = not constants.SHOW_SAME_ROOM_OSD + self._playerPaused = True if constants.SYNC_ON_PAUSE and self.getUsername() <> setBy: self.setPosition(self.getGlobalPosition()) self._playerCommand(self._player.setPaused, True) From 5dc2ca0da834804a35f00ec2f8c7fe74dc5d7151 Mon Sep 17 00:00:00 2001 From: Peter Marheine Date: Thu, 15 Oct 2015 19:34:55 -0600 Subject: [PATCH 3/6] Make player command delay configurable. --- syncplay/client.py | 2 +- syncplay/constants.py | 1 + syncplay/messages.py | 3 +++ syncplay/ui/ConfigurationGetter.py | 17 ++++++++++++++--- syncplay/ui/GuiConfiguration.py | 13 +++++++++++++ 5 files changed, 32 insertions(+), 4 deletions(-) diff --git a/syncplay/client.py b/syncplay/client.py index 22da494..df0d668 100644 --- a/syncplay/client.py +++ b/syncplay/client.py @@ -178,7 +178,7 @@ class SyncplayClient(object): def updatePlayerStatus(self, paused, position, cookie=None): # Ignore status report if the cookie is stale if cookie is not None and \ - cookie < constants.PLAYER_COMMAND_DELAY + self._lastPlayerCommand: + cookie < self._lastPlayerCommand + self._config['playerCommandDelay']: return position -= self.getUserOffset() diff --git a/syncplay/constants.py b/syncplay/constants.py index a39ec6a..eadacac 100644 --- a/syncplay/constants.py +++ b/syncplay/constants.py @@ -49,6 +49,7 @@ SERVER_STATE_INTERVAL = 1 WARNING_OSD_MESSAGES_LOOP_INTERVAL = 1 AUTOPLAY_DELAY = 3.0 SYNC_ON_PAUSE = True # Client seek to global position - subtitles may disappear on some media players +DEFAULT_PLAYER_COMMAND_DELAY = 0.05 # Options for the File Switch feature: FOLDER_SEARCH_TIMEOUT = 60.0 # Secs - How long to wait until searches in folder to update cache are aborted (may be longer than this if hard drive needs to spin up) diff --git a/syncplay/messages.py b/syncplay/messages.py index 7abe1ad..ff71183 100755 --- a/syncplay/messages.py +++ b/syncplay/messages.py @@ -194,6 +194,9 @@ en = { "forceguiprompt-label" : "Don't always show the Syncplay configuration window", # (Inverted) "nostore-label" : "Don't store this configuration", # (Inverted) "showosd-label" : "Enable OSD Messages", + "playercommanddelay-title" : "Player latency compensation", + "playercommanddelay-label" : "Seconds to ignore player status after commands", + "playercommanddelay-tooltip" : "Larger values are less likely to spuriously (un)pause but tend to sync less accurately.", "showosdwarnings-label" : "Include warnings (e.g. when files are different, users not ready)", "showsameroomosd-label" : "Include events in your room", diff --git a/syncplay/ui/ConfigurationGetter.py b/syncplay/ui/ConfigurationGetter.py index 0fa6284..b27af5c 100755 --- a/syncplay/ui/ConfigurationGetter.py +++ b/syncplay/ui/ConfigurationGetter.py @@ -63,7 +63,8 @@ class ConfigurationGetter(object): "showSameRoomOSD" : True, "showNonControllerOSD" : False, "showContactInfo" : True, - "showDurationNotification" : True + "showDurationNotification" : True, + "playerCommandDelay": constants.DEFAULT_PLAYER_COMMAND_DELAY } self._defaultConfig = self._config.copy() @@ -117,11 +118,21 @@ class ConfigurationGetter(object): "rewindThreshold", "fastforwardThreshold", "autoplayMinUsers", + "playerCommandDelay", ] self._iniStructure = { "server_data": ["host", "port", "password"], - "client_settings": ["name", "room", "playerPath", "perPlayerArguments", "slowdownThreshold", "rewindThreshold", "fastforwardThreshold", "slowOnDesync", "rewindOnDesync", "fastforwardOnDesync", "dontSlowDownWithMe", "forceGuiPrompt", "filenamePrivacyMode", "filesizePrivacyMode", "unpauseAction", "pauseOnLeave", "readyAtStart", "autoplayMinUsers", "autoplayInitialState", "mediaSearchDirectories"], + "client_settings": ["name", "room", "playerPath", + "perPlayerArguments", "slowdownThreshold", + "rewindThreshold", "fastforwardThreshold", + "slowOnDesync", "rewindOnDesync", + "fastforwardOnDesync", "dontSlowDownWithMe", + "forceGuiPrompt", "filenamePrivacyMode", + "filesizePrivacyMode", "unpauseAction", + "pauseOnLeave", "readyAtStart", "autoplayMinUsers", + "autoplayInitialState", "mediaSearchDirectories", + "playerCommandDelay"], "gui": ["showOSD", "showOSDWarnings", "showSlowdownOSD", "showDifferentRoomOSD", "showSameRoomOSD", "showNonControllerOSD", "showDurationNotification"], "general": ["language", "checkForUpdatesAutomatically", "lastCheckedForUpdates"] } @@ -425,4 +436,4 @@ class SafeConfigParserUnicode(SafeConfigParser): if (value is not None) or (self._optcre == self.OPTCRE): key = " = ".join((key, unicode(value).replace('\n', '\n\t'))) fp.write("%s\n" % key) - fp.write("\n") \ No newline at end of file + fp.write("\n") diff --git a/syncplay/ui/GuiConfiguration.py b/syncplay/ui/GuiConfiguration.py index bfa92ec..ad8f80c 100644 --- a/syncplay/ui/GuiConfiguration.py +++ b/syncplay/ui/GuiConfiguration.py @@ -365,6 +365,8 @@ class ConfigDialog(QtGui.QDialog): widget.setChecked(True) elif isinstance(widget, QLineEdit): widget.setText(self.config[valueName]) + elif isinstance(widget, QDoubleSpinBox): + widget.setValue(self.config[valueName]) def saveValues(self, widget): valueName = str(widget.objectName()) @@ -682,6 +684,10 @@ class ConfigDialog(QtGui.QDialog): self.rewindCheckbox.setObjectName("rewindOnDesync") self.fastforwardCheckbox = QCheckBox(getMessage("fastforwardondesync-label")) self.fastforwardCheckbox.setObjectName("fastforwardOnDesync") + self.commandDelaySpinbox = QDoubleSpinBox() + self.commandDelaySpinbox.setObjectName("playerCommandDelay") + self.commandDelaySpinbox.setMaximum(10) + self.commandDelaySpinbox.setSingleStep(.01) self.desyncSettingsLayout = QtGui.QGridLayout() self.desyncSettingsLayout.setSpacing(2) @@ -710,10 +716,17 @@ class ConfigDialog(QtGui.QDialog): self.othersyncSettingsLayout.setAlignment(Qt.AlignLeft) self.othersyncSettingsLayout.addWidget(self.fastforwardCheckbox, 3, 0,1,2, Qt.AlignLeft) + self.playerLatencyGroup = QtGui.QGroupBox(getMessage("playercommanddelay-title")) + self.playerLatencyLayout = QtGui.QHBoxLayout() + self.playerLatencyGroup.setLayout(self.playerLatencyLayout) + self.playerLatencyLayout.addWidget(self.commandDelaySpinbox) + self.playerLatencyLayout.addWidget(QLabel(getMessage("playercommanddelay-label"))) + self.othersyncSettingsGroup.setLayout(self.othersyncSettingsLayout) self.othersyncSettingsGroup.setMaximumHeight(self.othersyncSettingsGroup.minimumSizeHint().height()) self.syncSettingsLayout.addWidget(self.othersyncSettingsGroup) self.syncSettingsLayout.addWidget(self.desyncSettingsGroup) + self.syncSettingsLayout.addWidget(self.playerLatencyGroup) self.syncSettingsFrame.setLayout(self.syncSettingsLayout) self.desyncSettingsGroup.setMaximumHeight(self.desyncSettingsGroup.minimumSizeHint().height()) self.syncSettingsLayout.setAlignment(Qt.AlignTop) From 16d53345a96828f0bb73414e80785fe9f72ceb8c Mon Sep 17 00:00:00 2001 From: Peter Marheine Date: Thu, 15 Oct 2015 20:11:54 -0600 Subject: [PATCH 4/6] Add cookie support to all player modules. --- syncplay/players/basePlayer.py | 4 ++-- syncplay/players/mpc.py | 12 +++++++----- syncplay/players/mplayer.py | 4 ++-- syncplay/players/mpv.py | 8 +++++--- syncplay/players/vlc.py | 10 +++++++--- 5 files changed, 23 insertions(+), 15 deletions(-) diff --git a/syncplay/players/basePlayer.py b/syncplay/players/basePlayer.py index c8fdcc2..37b81d7 100644 --- a/syncplay/players/basePlayer.py +++ b/syncplay/players/basePlayer.py @@ -6,7 +6,7 @@ class BasePlayer(object): execute updatePlayerStatus(paused, position) on client Given the arguments: boolean paused and float position in seconds ''' - def askForStatus(self): + def askForStatus(self, cookie=None): raise NotImplementedError() ''' @@ -121,4 +121,4 @@ class DummyPlayer(BasePlayer): @staticmethod def getPlayerPathErrors(playerPath, filePath): - return None \ No newline at end of file + return None diff --git a/syncplay/players/mpc.py b/syncplay/players/mpc.py index 8b2c938..440ee43 100644 --- a/syncplay/players/mpc.py +++ b/syncplay/players/mpc.py @@ -420,20 +420,22 @@ class MPCHCAPIPlayer(BasePlayer): return self._mpcApi.lastFilePosition @retry(MpcHcApi.PlayerNotReadyException, constants.MPC_MAX_RETRIES, constants.MPC_RETRY_WAIT_TIME, 1) - def askForStatus(self): + def askForStatus(self, cookie=None): if self._mpcApi.filePlaying and self.__preventAsking.wait(0) and self.__fileUpdate.acquire(0): self.__fileUpdate.release() position = self.__getPosition() paused = self._mpcApi.isPaused() position = float(position) if self.__preventAsking.wait(0) and self.__fileUpdate.acquire(0): - self.__client.updatePlayerStatus(paused, position) + self.__client.updatePlayerStatus(paused, position, cookie=cookie) self.__fileUpdate.release() return - self.__echoGlobalStatus() + self.__echoGlobalStatus(cookie) - def __echoGlobalStatus(self): - self.__client.updatePlayerStatus(self.__client.getGlobalPaused(), self.__client.getGlobalPosition()) + def __echoGlobalStatus(self, cookie): + self.__client.updatePlayerStatus(self.__client.getGlobalPaused(), + self.__client.getGlobalPosition(), + cookie=cookie) def __forcePause(self): for _ in xrange(constants.MPC_MAX_RETRIES): diff --git a/syncplay/players/mplayer.py b/syncplay/players/mplayer.py index dd35762..65124a5 100644 --- a/syncplay/players/mplayer.py +++ b/syncplay/players/mplayer.py @@ -72,14 +72,14 @@ class MplayerPlayer(BasePlayer): self.reactor.callLater(0, self._client.initPlayer, self) self._onFileUpdate() - def askForStatus(self): + def askForStatus(self, cookie=None): self._positionAsk.clear() self._pausedAsk.clear() self._getPaused() self._getPosition() self._positionAsk.wait() self._pausedAsk.wait() - self._client.updatePlayerStatus(self._paused, self._position) + self._client.updatePlayerStatus(self._paused, self._position, cookie=cookie) def _setProperty(self, property_, value): self._listener.sendLine("set_property {} {}".format(property_, value)) diff --git a/syncplay/players/mpv.py b/syncplay/players/mpv.py index ff52af2..1130ca7 100644 --- a/syncplay/players/mpv.py +++ b/syncplay/players/mpv.py @@ -154,14 +154,16 @@ class NewMpvPlayer(OldMpvPlayer): else: self._paused = self._client.getGlobalPaused() - def askForStatus(self): + def askForStatus(self, cookie=None): self._positionAsk.clear() self._pausedAsk.clear() self._getPaused() self._getPosition() self._positionAsk.wait(constants.MPV_LOCK_WAIT_TIME) self._pausedAsk.wait(constants.MPV_LOCK_WAIT_TIME) - self._client.updatePlayerStatus(self._paused if self.fileLoaded else self._client.getGlobalPaused(), self.getCalculatedPosition()) + self._client.updatePlayerStatus(self._paused if self.fileLoaded else self._client.getGlobalPaused(), + self.getCalculatedPosition(), + cookie=cookie) def _preparePlayer(self): if self.delayedFilePath: @@ -219,4 +221,4 @@ class NewMpvPlayer(OldMpvPlayer): if self.fileLoaded == True and self.lastLoadedTime != None and time.time() > (self.lastLoadedTime + constants.MPV_NEWFILE_IGNORE_TIME): return True else: - return False \ No newline at end of file + return False diff --git a/syncplay/players/vlc.py b/syncplay/players/vlc.py index 331fcbf..70a9dd3 100644 --- a/syncplay/players/vlc.py +++ b/syncplay/players/vlc.py @@ -84,16 +84,20 @@ class VlcPlayer(BasePlayer): self.setPaused(self._client.getGlobalPaused()) self.setPosition(self._client.getGlobalPosition()) - def askForStatus(self): + def askForStatus(self, cookie=None): self._filechanged = False self._positionAsk.clear() self._pausedAsk.clear() self._listener.sendLine(".") if self._filename and not self._filechanged: self._positionAsk.wait(constants.PLAYER_ASK_DELAY) - self._client.updatePlayerStatus(self._paused, self.getCalculatedPosition()) + self._client.updatePlayerStatus(self._paused, + self.getCalculatedPosition(), + cookie=cookie) else: - self._client.updatePlayerStatus(self._client.getGlobalPaused(), self._client.getGlobalPosition()) + self._client.updatePlayerStatus(self._client.getGlobalPaused(), + self._client.getGlobalPosition(), + cookie=cookie) def getCalculatedPosition(self): if self._lastVLCPositionUpdate is None: From 4c7c526b8dcbb88d79ddf99108a8514419fbd096 Mon Sep 17 00:00:00 2001 From: Peter Marheine Date: Fri, 16 Oct 2015 20:36:37 -0600 Subject: [PATCH 5/6] Add missing GUI saving for command delay Also adjust the default command delay step to 100ms. --- syncplay/ui/GuiConfiguration.py | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/syncplay/ui/GuiConfiguration.py b/syncplay/ui/GuiConfiguration.py index ad8f80c..26e7282 100644 --- a/syncplay/ui/GuiConfiguration.py +++ b/syncplay/ui/GuiConfiguration.py @@ -389,6 +389,8 @@ class ConfigDialog(QtGui.QDialog): self.config[radioName] = radioValue elif isinstance(widget, QLineEdit): self.config[valueName] = widget.text() + elif isinstance(widget, QDoubleSpinBox): + self.config[valueName] = widget.value() def connectChildren(self, widget): widgetName = str(widget.objectName()) @@ -687,7 +689,7 @@ class ConfigDialog(QtGui.QDialog): self.commandDelaySpinbox = QDoubleSpinBox() self.commandDelaySpinbox.setObjectName("playerCommandDelay") self.commandDelaySpinbox.setMaximum(10) - self.commandDelaySpinbox.setSingleStep(.01) + self.commandDelaySpinbox.setSingleStep(.1) self.desyncSettingsLayout = QtGui.QGridLayout() self.desyncSettingsLayout.setSpacing(2) @@ -1009,4 +1011,4 @@ class ConfigDialog(QtGui.QDialog): self.processWidget(self, lambda w: self.loadTooltips(w)) self.processWidget(self, lambda w: self.loadValues(w)) self.processWidget(self, lambda w: self.connectChildren(w)) - self.populateEmptyServerList() \ No newline at end of file + self.populateEmptyServerList() From 094a204d9e2d240dd601447862b8d2fcab64b469 Mon Sep 17 00:00:00 2001 From: Peter Marheine Date: Thu, 22 Oct 2015 19:54:14 -0600 Subject: [PATCH 6/6] Update player location on server seek. --- syncplay/client.py | 10 ++++++++-- 1 file changed, 8 insertions(+), 2 deletions(-) diff --git a/syncplay/client.py b/syncplay/client.py index df0d668..0adf967 100644 --- a/syncplay/client.py +++ b/syncplay/client.py @@ -175,10 +175,15 @@ class SyncplayClient(object): seeked = _playerDiff > constants.SEEK_THRESHOLD and _globalDiff > constants.SEEK_THRESHOLD return pauseChange, seeked + def _ignoringPlayerStatus(self, cookie=None): + if cookie is None: + cookie = time.time() + return cookie < self._lastPlayerCommand + self._config['playerCommandDelay'] + def updatePlayerStatus(self, paused, position, cookie=None): # Ignore status report if the cookie is stale - if cookie is not None and \ - cookie < self._lastPlayerCommand + self._config['playerCommandDelay']: + if self._ignoringPlayerStatus(cookie): + self.ui.showDebugMessage('Ignoring stale player status with cookie {}'.format(cookie)) return position -= self.getUserOffset() @@ -515,6 +520,7 @@ class SyncplayClient(object): def setPosition(self, position): if self._lastPlayerUpdate: self._lastPlayerUpdate = time.time() + self._playerPosition = position position += self.getUserOffset() if self._player and self.userlist.currentUser.file: if position < 0: