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

Request/Suggestion: Provide information about playlist limitations in documentation #655

Open
PuffyRainbowCloud opened this issue Jan 2, 2024 · 7 comments
Assignees

Comments

@PuffyRainbowCloud
Copy link

Throughout my time with SyncPlay I have had issues getting playlists to load correctly. Some are rejected by the server, and some even cause Python to throw errors. By testing with various attributes, I've been able to narrow down two major limitations to the playlists:

  • A maximum of 10,000 characters
  • A maximum of 150 lines

Anything beyond that results in some form of error or incorrect behaviour. This isn't an issue per se. It's more of an observation. I think it could be valuable to include it somewhere in the documentation if it isn't already. I never found any mention of it when trying to work it out myself. I doubt that I'm the first or last person to use SyncPlay in a scenario where playlists routinely exceed these limitations, after all.

@Et0h
Copy link
Contributor

Et0h commented Jan 2, 2024

I've now added information to the FAQ/troubleshooting page at https://syncplay.pl/guide/trouble/

Those playlist limits are actually server-side as part of the playlistIsValid function in utils.py that is references in server.py. The function draww on the constant values PLAYLIST_MAX_ITEMS and PLAYLIST_MAX_CHARACTERS from constants.py.

This means that if you want to allow longer characters/lines you can just change these constant values on your server, which is easy if you are running direct form Python but not possible for compiled versions of the server.

I basically set those thresholds as a somewhat arbitrary large numbers because there needed to be some upper bound to mitigate abuse and I couldn't think of anyone wanting anything close to 10,000 characters or 150 lines so thought it a safe limit to chose with some margin.

I set these limits 7 years ago, and this is the first time I have heard a complaint that the limits are overly restrictive. As such, for most users the current limits seem to not be an issue.

Syncplay is generally designed to accommodate the most common use cases rather than every possible edge case, but I would be interested in knowing what use cases people have that would require a higher limit and what that higher limit would need to be to accommodate that use case.

@Et0h Et0h self-assigned this Jan 2, 2024
@selurvedu
Copy link

Thanks a lot! I noticed this back in April and made a few examples of playlists to reproduce it, but I couldn't narrow it down to exact numbers and hence did not report it.

@selurvedu
Copy link

Some playlist edits simply caused the changes to get discarded and the client to get disconnected. Either the edit attempt caused the server to restart, or the server rejected the edit and kicked the client, I can't remember right now.

Some other edits caused the server to crash after saving them into the database, effectively breaking it altogether, as it kept crashing on restart until the database got fixed or removed manually.

I had an example of a playlist that made both cases reproducible, but it may take a while once I locate it. I needed to export every playlist by hand daily because the crash could happen anytime, and I didn't want my playlist to get lost again, so it should be there somewhere among over a hundred of versions I currently have.

@Et0h
Copy link
Contributor

Et0h commented Jan 8, 2024

@selurvedu: It's very interesting to hear that Syncplay might be allowing playlists that are too long for the server-side database on custom servers. This might indicate a lower threshold is warranted. It would be interesting to hear what that lower threshold ought to be.

When lowering it, I could also look into making there be a better error message for overly long playlists but first I'd want to be sure of how Syncplay should handle overly long playlists (in terms of entries or characters). If someone proposes a playlist that is too long, would it be better to reject the whole playlist or to trim it down to size?

@0fbcb238c0
Copy link

Would it be possible to at least output a message so the user is aware of why the playlist is not loading?

@selurvedu
Copy link

If someone proposes a playlist that is too long, would it be better to reject the whole playlist or to trim it down to size?

IMO it should give the user a warning that the playlist is too long, and give the user a chance to save their rejected playlist. I lost some edits quite a few times while e.g. adding many YT links in a row, so I had to find and re-add them again.

@Et0h
Copy link
Contributor

Et0h commented Sep 28, 2024

Would it be possible to at least output a message so the user is aware of why the playlist is not loading?

My plan is that once I get more details on what threshold causes issues in the server-side database I will set the limit to that then add an appropriate error message.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

4 participants