added a rewind on desync option

[blaenk]

Hey, I love Syncplay. Thanks for making it :)

I use it often with friends, but the problem is that I have a pretty slow connection. If anyone on my LAN does any minor thing, it'll lag my connection to syncplay. This causes frequent slowdowns and rewinds, even though as far as I know we're still in sync (it's just interpreted as desync due to the lag).

I was glad to see an option for 'slow on desync' in 1.2.7 so that we could disable that, but the most jarring of all to us is rewind on desync. Just yesterday, for example, I was lagging so bad that all three of us who were watching a 1 hr 48 minute movie ended up taking much longer to actually finish it because we all kept getting rewinded since they were misinterpreted as being out of sync with me (or vice versa) because of the lag on my connection.

So I added a 'rewind on desync' option. I've tried it so far (by downloading something myself while syncplaying, thereby lagging myself) and it seems to work perfectly.

I think it'd be nice to have this option in Syncplay, as it would complement the 'slow on desync' option well, and it took only a little bit of code to implement (mostly mirroring the 'slow on desync' implementation).

My friends and I are going to be using this modification/patch as sync playing can be unbearable (on slow connections) without it. I just figured I'd attempt to contribute it back.

Thanks again!

[Uriziel]

This seems rather like an issue with protocol and the solution is a workaround. I'll leave it for @Et0h (currently - the GUI guy) to decide.

But anyway, thanks for your contribution. If you'll get any ideas about how Syncplay should work feel free to make a ticket (welcome even without a patch :P)

[Uriziel]

I changed my mind. I'm against this change since rewinding due to time difference is the last resort of keeping people in sync.

The real problem lies somewhere else, I believe. The issue is not that your connection is slow (huge latency), but the spikes (jitter) between "Status messages".

System already takes your ping into account, but doesn't do that perfectly, I expect to fix that in few days.

What's more, when your connection spikes the data that was sent over should be invalidated, since it's outdated and that's what causing rewinds or slowdowns even though you expect to be in sync.

I'm going to fix that by ignoring the content of messages which RTT double the average RTT unless they force pause or seek. This is fine since status messages are just a tool to see if anybody is slowing down over some period of time, missing one or two won't hurt.

The increased latency will also be taken into account (with exponential moving average), so if the issue with your connection gets prolonged your status messages will get back to normal over few seconds of time.

Given all that I don't think this change is necessary apart from very unique usercases. I don't mind adding new config value that can be set manually, they're pretty always fine and welcome, but 👎 for leaving the option in the open in GUI. We added disabling of slowdown in there because it causes some performance issues and/or frame step in some players (MPC-HC). This is not the case here.

The option should be either moved to "advanced" part of the config (which doesn't exist at this point) that's folded up by default or available just through the config file.

Feel free to provide your opinion on either of the matters (solution or PR).

[blaenk]

Fair enough. It was the first thing I thought to change in response to the very bad experience with syncplay that time, I just wanted to get it out of the way before the movie finished so that we could finish it at all haha. I didn't spend time to give a proper look at the source.

Today I skimmed through what I thought were parts relevant to the issue, and I did notice that you were indeed taking the ping into account already (and using an EMA). I also did notice that in master (I was running an earlier release before), saturating my connection -- once reaching equilibrium -- rarely caused any problems (i.e., rewinds) in syncplay. It only sometimes occurred, indeed, during spikes, such as at the initiation of a download.

I guess I'm not really thinking clearly about it but can you please elucidate on situations in which desyncs can occur? I expect there are actual situations, I guess I'm just not very familiar with this sort of thing. My main concern was that, in my view (without the answer to that question), the desync detection ratio would be more false positives than actual desyncs.

I guess the naive and restricted way I'm thinking of it is, if everyone is at 0:00, and someone presses play, assuming that event gets disseminated to everyone in a timely manner, wouldn't they thereafter stay in sync and so have no reason to keep checking very frequently to make sure they're still in sync? Can the media player not be trusted to play at the correct speed? Is it because, perhaps, some watchers might lag during playback of difficult-to-decode content (e.g., 1080p/10bit) ? From what I've gathered, media playback synchronization within a player (i.e., audio/video) is difficult on its own, seems to me like it'd be much more complicated on top of another layer and over a network. I'm not saying it shouldn't be done (after all, that's the point of this program), I'm just genuinely curious about example situations in which desync can occur (where slow or rewind can be used to resync).

I'm going to fix that by ignoring the content of messages which RTT double the average RTT unless they force pause or seek. This is fine since status messages are just a tool to see if anybody is slowing down over some period of time, missing one or two won't hurt.

Seems like a logical solution to me!

The increased latency will also be taken into account (with exponential moving average), so if the issue with your connection gets prolonged your status messages will get back to normal over few seconds of time.

Is master already doing this? I did notice the EMA code, and testing earlier (like I mentioned above) I did notice that a saturated connection didn't cause (or rarely did) rewinds/slow downs in syncplay, unlike older versions.

It seems to me like these two things should mitigate these issues just fine.

As for this PR, I'm fine with whatever you decide. A manual configuration option sounds fine to me, in case someone finds themselves in an edge case and wants a last resort. But if you'd rather not even have that, it's fine by me. Just glad the issue is recognized and being addressed!

Thanks again.

[Uriziel]

There are few cases in which desync can occur. The main problem is someone's player lagging behind under heavy load, as you mentioned. Apart from that there are cases in which your player will framestep (i.e. mpc-hc while changing the displays) or do some sorts of other magic, happens from time to time. Syncplay can be also used with streams (like Youtube or incomplete downloads) that can also fail in certain situations.

The current way the protocol works with "State" commands is probably relic of first releases, and could be changed to something more sane, but it wasn't a priority since it is working fine. Unfortunately I'm spending too little time on developing Syncplay recently (maybe because it works :D).

Yeah, the EMA is already calculated, but I felt like explaining it further so other devs won't complain about possibility of creating a new issue (sup @Et0h)

I'm totally fine with the config option for disabling rewinds, so that probably gets accepted, but I'll wait for @Et0h's opinion on changes with GUI.

Thanks for your contribution. (I'm looking forward to the next time :])

[Et0h]

I have nothing against having an option for disabling rewind on desync in the GUI as there are circumstances where it could be quite useful. However, I think the labels should be reworded to something like "slow down on minor desync" and "rewind on major desync" so that users know that they are not mutually exclusive and that checking them both would not result in the video being both rewound and slowed down at the same time.

P.S. Thank you @blaenk for promoting and contributing to Syncplay.

[blaenk]

There are few cases in which desync can occur. The main problem is someone's player lagging behind under heavy load, as you mentioned.

Ah okay, makes sense.

Apart from that there are cases in which your player will framestep (i.e. mpc-hc while changing the displays) or do some sorts of other magic, happens from time to time.

Indeed! I had noticed that recently and I did wonder if the stream was still in sync so I always paused and resumed (thinking that'd resync) after switching displays. Good to know this is already taken into consideration.

The current way the protocol works with "State" commands is probably relic of first releases, and could be changed to something more sane, but it wasn't a priority since it is working fine. Unfortunately I'm spending too little time on developing Syncplay recently (maybe because it works :D).

I agree, if it works mostly fine, no reason to drastically change it. That said, given I'm new to network synchronization I'm not sure what an alternative to "State" commands would be. That had always been what I naively envisioned as a solution to the problem of playback synchronization.

However, I think the labels should be reworded to something like "slow down on minor desync" and "rewind on major desync" so that users know that they are not mutually exclusive and that checking them both would not result in the video being both rewound and slowed down at the same time.

I agree wholeheartedly; I hadn't thought of how ambiguous those two options could appear if worded that way. That seems like a good way to disambiguate.

Glad I could help in some way guys; I love this program. I was actually motivated to write something similar right into MPC-HC itself, but then one of the MPC-HC devs told me that this (Syncplay) already existed, and it works with even more players, and is cross-platform! Friends and I use it all the time, so I'm glad to contribute in some way.

[Et0h]

I have included the checkbox in the configuration GUI, but I relabelled it "Rewind on major desync" and made it only appear if the SHOW_REWIND_ON_DESYNC_CHECKBOX constant is changed to True. This makes the option easy to toggle for those who need it, but not appear for those who don't. Hopefully with Uriziel's improvements to jitter handling not many people will need it in the future, but it is still a nice feature to have.