Conversation
|
Hi, I'm busy with other things at the moment it will take me some time to go through all this, but thanks for helping to progress this long-requested feature (and for you following Syncplay naming conventions, etc). Based in part on my experience from shared playlists and a quick look at the code my initial thoughts in terms of checking to ensure things work as expected are (in no particular order):
In my recollection MPC-HC and MPC-BE do support variable speeds, it is just they sometimes had a short pause when the speed changed so using it to handle slowdown would be counterproductive. It might be possible to support speed changes on MPC-HC/BE just not using it for slowdown support - someone would need to test it. At present your code uses the in-player speed change to trigger changes in speed. It might be better to get people to control speed via Syncplay GUI and CLI as I'm not sure how easy it is to reliably detect a manual change in speed compared to a change which might happen when you change file for example. However, if it is reliable then allowing it to change via media player would be in line with our ability for people to seek via the player. |
|
Thanks for the detailed review @Et0h! I've gone through your checklist systematically, added comprehensive unit tests (now totaling 51), and spent significant time smoothing out interactions between different player engines (specifically mpv and VLC). Here is the status on the specific points you raised:
Feedback Loops & Player FixesWhile validating the above, I tried the changes on VLC as well, and found and fixed several specific synchronization issues, particularly when syncing between mpv and VLC:
I have verified these scenarios with mpv <-> VLC cross-sync, fast consecutive changes, and lag/drift simulation. |
|
Nice work. You now probably know some parts of the Syncplay code better than me. Testing is very valuable so I appreciate you introducing a methodical approach to it. Trying to deal with VLC quirks can be its own unique type of a headache and can require a bit of fudging - that's why we advise people to use mpv for the most reliable Syncplay experience. I'll give some thought on how best to handle inter-version compatibility in terms of giving warnings if the server or other clients don't support speed changes. My inclination is to use the features feature as that is what I designed it for. |
… sync - Remove syncplay_speed_sync.7z binary from repository - Extract hardcoded "Current speed" string to messages_en.py - Downgrade speed-sync-not-supported message from error to debug - Bump SPEED_SYNC_MIN_VERSION from 1.7.3 to 1.7.5 - Add updateSpeed() to GUI and console UI so speedInput reflects remote speed changes and local setSpeed() calls - Update test to match new minimum version Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>

Fixes #115 , #339 , #406
Summary
Player Support
speedSupported = Falsein current API implementation)Files Changed
server.py: Room/ControlledRoom store speed; Watcher propagates speed changes.protocols.py: Added Speed field in State message (client + server).client.py: Global speed state, dead-reckoning, slowdown relative to room speed.players/*.py: Added thread-safe speed detection via properties/events for mpv, VLC, and MPlayer.resources/lua/*: Updated Lua scripts to report speed/rate in status responses.constants.py:DEFAULT_PLAYBACK_SPEED = 1.0messages_en.py: Addedspeed-change-notificationmessage.Test plan
I have verified the following scenarios:
tests/test_speed_sync.pyand all tests pass.Didn't verify VLC or MPlayer as none of my friends use it and I'm not familiar with how they work at all, so I'd appreciate it if someone can verify these players as well.