calzoneman / sync

Node.JS Server and JavaScript/HTML Client for synchronizing online media
Other
1.46k stars 235 forks source link

[Long term] Refactor player structure #794

Open calzoneman opened 5 years ago

calzoneman commented 5 years ago

https://github.com/calzoneman/sync/commit/dfb7177a6d926476c03425acaeabc29c2bf1e177 adds a quick fix for a bug causing the Dailymotion player to get corrupted if API calls were made after calling load() to load a new video once the previous one finished, but before the video had fully loaded.

This exposes a more general problem which is that the player code is not written to gracefully mediate between incoming socket.io video update frames and whatever asynchronous events the player is emitting to say it's ready. Most video types use an asynchronous constructor to wait for the API to load before making any calls, but the video update code mostly just yolo-calls play/pause/seekTo synchronously and hopes for the best (which the player implementations guard against with "ready" flags and similar kludges).

There are various other non-ideal aspects of the way the player logic is handled today that I won't go into details here, but overall it might be time to rewrite the whole thing.

calzoneman commented 5 years ago

It'd also be nice to rewrite it in ES2015 in the process since the player code is the only remaining part of the codebase that uses CoffeeScript.

Xaekai commented 2 years ago

Now that the player has been updated in the pending update, this should be revisited.