Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Port more video control settings (Undo Seek + Seek to specific time) #23

Closed
yuroyami opened this issue Jul 12, 2022 · 7 comments
Closed
Assignees
Labels
feature port Porting a feature from the PC version (Mimicking functionality)

Comments

@yuroyami
Copy link
Owner

yuroyami commented Jul 12, 2022

Undo Seek: There should be no more "Oups". When a user mistakingly seeks, he should be able to revert the action.

Seek to specific time: Ability to enter a specific time to seek to without looking for it with the seek bar.

@yuroyami yuroyami added the feature port Porting a feature from the PC version (Mimicking functionality) label Jul 12, 2022
@yuroyami yuroyami self-assigned this Jul 12, 2022
@Et0h
Copy link

Et0h commented Jul 13, 2022

You're free to port the offset feature but be aware that Syncplay PC consider it a deprecated feature. We now just advise everyone to have the same file or at least an alternative of the same duration.

Offset kinda made some sense as a feature when Syncplay was first released in around 2012 and was focused on just syncing one file. At that time opening and ending files were sometimes stored externally and some players didn't handle that properly meaning the was a fixed knowable offset to apply to fix the playback. However it was always a hassle to use, not least because it was never intuitive whether +1 second offset was for when your video was a second ahead or a second behind other people. It was also not intuitive whether the offset was reset or maintained when you changed files.

Now we have shared playlists it is even worse in terms of how it's supposed to be compatible with automatic file switching and whether the offset should apply to all videos or just the current one, etc. Furthermore shared playlists are built around one filename per synced video so many will just get the same version of a file to avoid having to rename everything to get that feature to work.

Coding wise offset adds what could be seen as unnecessary complexity.

@yuroyami
Copy link
Owner Author

yuroyami commented Jul 13, 2022

@Et0h I was wondering what the offset would be for other than syncing videos that appear to be a bit different (such as a movie from a BluRay release, compared to a movie from a WEB-DL release) but even so it didn't make much sense to me. Now that I know what it was supposed to be a solution for, it only makes sense to deprecate it. If so, deprecation means that it's no longer recommended for use. Coding-wise it would indeed complicate things, I believe that I should focus on other important features to port since my end goal is to make the perfect client that has the same mirrorcopy functionality as Syncplay PC.

By the time most features get ported and the app reaches a stable release, Syncplay will be uploaded to Google Store to test it out with the masses. For now, Syncplay PC and Android show amazing promise and interoperability.

@yuroyami yuroyami changed the title Port more video control settings (Set Offset + Undo Seek + Seek to specific time) Port more video control settings (Undo Seek + Seek to specific time) Jul 13, 2022
@Et0h
Copy link

Et0h commented Jul 13, 2022

Syncplay/syncplay#435 provides a discussion of this feature and its deprecation. At least one group of users said last year that they use the feature "all the time" (but don't use the shared playlists) to accommodate different releases of a video catered for different screen resolutions and file sizes. You could still implement the feature for such groups if you want. However there is no need to cater for every use case, so you could decide to make it a low priority or just beyond what your software supports.

@yuroyami
Copy link
Owner Author

yuroyami commented Jul 13, 2022

It is still possible to implement the feature, and I believe that the announced video position would not change even after setting the offset, correct ? If so, despite it being a bit complicated to implement, it can be added last after porting all non-deprecated features. I will end up bored and idle-handed anyhoo. I separated the issue as a feature port of low priority.

Btw, I stumbled upon something called "room persistence", I might have had a faint idea of what it is, but I guess it's just server-side ?

@Et0h
Copy link

Et0h commented Jul 13, 2022

It is still possible to implement the feature, and I believe that the announced video position would not change even after setting the offset, correct ?

If I recall correctly (and its been a while since I've looked at that code in detail) the announced video position adjusts for the offset. This basically means that only the client with an offset set needs to know that it is using the offset feature. There is nothing in the protocol to state that the position is having an offset applied to it.

Btw, I stumbled upon something called "room persistence", I might have had a faint idea of what it is, but I guess it's just server-side ?

It's mostly just server side, not least to maintain backwards compatibility with older clients. There are some potentially low priority UI implications if you list what is happening in other rooms on non-public servers. The feature also explains the uiMode setting in the most recent version of the Syncplay protocol.

Room persistence allows for room playlist information to be retained on privately run servers even when nobody is in a room. The rooms are included in the list of who is playing what even when nobody is in them for as long as they have files in the shared playlist.

The related feature called room permanence allows for a list of rooms to be specified on the server which will then always be listed in the list of who is playing what even when the shared playlist is empty (especially useful for managed rooms where the name is important).

While both of these features are server side, they also have the effect of allowing rooms to appear in the list of who is playing what that don't have any actual users in them. The Syncplay client was never designed to handle empty rooms. To make the feature backwards compatible with older clients we therefore have to use blank dummy users to populate such rooms. Syncplay doesn't do this if uiMode is set to CLI (aka CONSOLE_UI_MODE) in client features and instead skips empty rooms. That's why uiMode is specified in the most recent versions of the Syncplay protocol.

Newer clients can detect when a room is actually empty and there is a GUI options to hide empty rooms from the list of who is playing what. Hiding empty rooms could be useful if you have many many persistent rooms but don't switch between them during a session which is a use case we've heard of although it's probably not a common use case.

All of this is irrelevant to those who use the official public servers as room persistence and permanence are for privately run servers, not servers with room isolation enabled and with a no logging policy. With room isolation you don't see what is happening in other rooms which therefore allow the room name to act as a kind of password. This is handled server-side.

@yuroyami
Copy link
Owner Author

yuroyami commented Jul 13, 2022

If I recall correctly (and its been a while since I've looked at that code in detail) the announced video position adjusts for the offset. This basically means that only the client with an offset set needs to know that it is using the offset feature. There is nothing in the protocol to state that the position is having an offset applied to it.

That only makes sense. Announcing video position that is affected by an applied offset beats the purpose of the latter.

Room persistence allows for room playlist information to be retained on privately run servers even when nobody is in a room. The rooms are included in the list of who is playing what even when nobody is in them for as long as they have files in the shared playlist.

The reason why I was interested to know what room permanence/persistence is, is that I might end up coding a Syncplay server for the Android environment where it can be launched with just one click, taking the same arguments Syncplay PC's server takes. Such option is pretty helpful for private servers where groups do not want to use the same playlist over and over upon entering a room.

I haven't come upon the uiMode feature yet (Or probably, I am just not remembering it or recognizing it). Syncplay Android for now supports input for private servers as well so it must be built to be capable of handling empty rooms nicely.

I have created a discussion on Syncplay PC's official repo to discuss a few other things.

@yuroyami
Copy link
Owner Author

Added more seek features in the latest version 0.11.0.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
feature port Porting a feature from the PC version (Mimicking functionality)
Projects
None yet
Development

No branches or pull requests

2 participants